Bug 113066 - [Qt][WK2] WebView's interactive property is not fully respected
Summary: [Qt][WK2] WebView's interactive property is not fully respected
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andras Becsi
URL:
Keywords:
Depends on:
Blocks: 110211
  Show dependency treegraph
 
Reported: 2013-03-22 08:40 PDT by Andras Becsi
Modified: 2013-04-08 04:12 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.96 KB, patch)
2013-03-22 08:40 PDT, Andras Becsi
no flags Details | Formatted Diff | Diff
Patch (4.15 KB, patch)
2013-03-22 11:05 PDT, Andras Becsi
no flags Details | Formatted Diff | Diff
Patch (4.08 KB, patch)
2013-03-25 10:35 PDT, Andras Becsi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andras Becsi 2013-03-22 08:40:19 PDT
[Qt][WK2] WebView's interactive property is not fully respected
Comment 1 Andras Becsi 2013-03-22 08:40:55 PDT
Created attachment 194557 [details]
Patch
Comment 2 Andras Becsi 2013-03-22 08:44:43 PDT
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 3 Jocelyn Turcotte 2013-03-22 08:57:50 PDT
Comment on attachment 194557 [details]
Patch

Do we disable pinching somehow already?
Comment 4 Andras Becsi 2013-03-22 11:05:14 PDT
Created attachment 194601 [details]
Patch
Comment 5 Andras Becsi 2013-03-22 11:06:53 PDT
(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 6 Jocelyn Turcotte 2013-03-25 08:15:41 PDT
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?
Comment 7 Andras Becsi 2013-03-25 09:14:07 PDT
(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.
Comment 8 Andras Becsi 2013-03-25 10:35:42 PDT
Created attachment 194884 [details]
Patch
Comment 9 Jocelyn Turcotte 2013-03-26 03:57:24 PDT
LGTM

Benjamin, could you have a look?
Comment 10 Andras Becsi 2013-04-03 03:19:54 PDT
Ping for review?
Comment 11 Benjamin Poulain 2013-04-05 12:25:50 PDT
Comment on attachment 194884 [details]
Patch

Good to go for WebKit2.
Comment 12 Andras Becsi 2013-04-08 04:11:55 PDT
Committed r147909: <http://trac.webkit.org/changeset/147909>
Comment 13 Andras Becsi 2013-04-08 04:12:33 PDT
Comment on attachment 194884 [details]
Patch

Clearing flags from attachment.