[Qt][WK2] WebView's interactive property is not fully respected

[Andras Becsi]

[Qt][WK2] WebView's interactive property is not fully respected

[Andras Becsi]

Created attachment 194557 [details]
Patch

[Andras Becsi]

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.

[Jocelyn Turcotte]

Comment on attachment 194557 [details]
Patch

Do we disable pinching somehow already?

[Andras Becsi]

Created attachment 194601 [details]
Patch

[Andras Becsi]

(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.

[Jocelyn Turcotte]

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?

[Andras Becsi]

(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.

[Andras Becsi]

Created attachment 194884 [details]
Patch

[Jocelyn Turcotte]

LGTM

Benjamin, could you have a look?

[Andras Becsi]

Ping for review?

[Benjamin Poulain]

Comment on attachment 194884 [details]
Patch

Good to go for WebKit2.

[Andras Becsi]

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

[Andras Becsi]

Comment on attachment 194884 [details]
Patch

Clearing flags from attachment.