Created attachment 194557 [details] Patch

Flickable also disables wheel scrolling if set to non-interactive, but disabling scrolling would mean that we would also ignore scroll requests coming from the web content which would not be a preferable behaviour.

Comment on attachment 194557 [details] Patch Do we disable pinching somehow already?

Created attachment 194601 [details] Patch

(In reply to comment #3) > (From update of attachment 194557 [details]) > Do we disable pinching somehow already? Indeed, that's also circumventing the Flickable, so we need to ignore requests.

Comment on attachment 194601 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194601&action=review > Source/WebKit2/UIProcess/qt/PageViewportControllerClientQt.cpp:433 > + if (!m_controller->allowsUserScaling() || !m_viewportItem->isInteractive()) An edge case would be if the user code sets interactive = false as a response of a certain pinch, or just randomly while a pinch is ongoing. In that case we should avoid inconsistent states, which I think could happen if we just stop listening in every callback from the recognizer. Handling this in the gesture recognizer itself would allow us to force a pinchGestureCancelled(), but this wouldn't allow us to easily send pinch gestures to the web page in the future if interactive == false. So that might not be better. Any idea?

(In reply to comment #6) > (From update of attachment 194601 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=194601&action=review > > > Source/WebKit2/UIProcess/qt/PageViewportControllerClientQt.cpp:433 > > + if (!m_controller->allowsUserScaling() || !m_viewportItem->isInteractive()) > > An edge case would be if the user code sets interactive = false as a response of a certain pinch, or just randomly while a pinch is ongoing. > In that case we should avoid inconsistent states, which I think could happen if we just stop listening in every callback from the recognizer. This corner case does not seem possible since we do not expose any gesture state in QML, thus there is no direct way to switch interactivity upon a pinch gesture. Indirectly it is possible to change interactivity in response to page size changes, but this does not only happen during pinch zoom. > > Handling this in the gesture recognizer itself would allow us to force a pinchGestureCancelled(), but this wouldn't allow us to easily send pinch gestures to the web page in the future if interactive == false. So that might not be better. > Any idea? I don't think changes in interactivity should force panGestureCancelled() since the gesture would not actually be cancelled it would only be ignored. Although the condition in pinchGestureEnded() has to be improved so that no such case can result in an inconsistent state.

Created attachment 194884 [details] Patch

LGTM Benjamin, could you have a look?

Ping for review?

Comment on attachment 194884 [details] Patch Good to go for WebKit2.

Committed r147909: <http://trac.webkit.org/changeset/147909>

Comment on attachment 194884 [details] Patch Clearing flags from attachment.