Bug 66599 – Chromium Mac: PDF: Scrollsbars should be visible when scrolling using the two finger gesture on Lion

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sailesh Agrawal]

Chromium Mac: PDF: Scrollsbars should be visible when scrolling using the two finger gesture on Lion

[Sailesh Agrawal]

Created attachment 104578 [details]
Patch

[Sailesh Agrawal]

This fixes the following chromium bug:
http://code.google.com/p/chromium/issues/detail?id=92606

[Sailesh Agrawal]

Ping!

[Nico Weber]

Comment on attachment 104578 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=104578&action=review

> Source/WebKit/chromium/src/WebViewImpl.cpp:615
> +

I'm not an expert in plugins, but I would've expected the flow to go "if plugin has touch focus, send to plugin, and only send to webview if plugin didn't handle it". Sending the event to two targets looks weird to me. But again, I'm probably not the right person to look at this.

[Sailesh Agrawal]

Comment on attachment 104578 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=104578&action=review

>> Source/WebKit/chromium/src/WebViewImpl.cpp:615
>> +
> 
> I'm not an expert in plugins, but I would've expected the flow to go "if plugin has touch focus, send to plugin, and only send to webview if plugin didn't handle it". Sending the event to two targets looks weird to me. But again, I'm probably not the right person to look at this.

Unfortunately this is not possible right now. Currently ScrollableArea::handleGestureEvent() just swallows all events.

There's a TODO in EventHandler::handleGestureEvent() to add proper hit testing. I think once this is done it'll be possible to go back and do this.

[Darin Fisher (:fishd, Google)]

Comment on attachment 104578 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=104578&action=review

>>> Source/WebKit/chromium/src/WebViewImpl.cpp:615
>>> +
>> 
>> I'm not an expert in plugins, but I would've expected the flow to go "if plugin has touch focus, send to plugin, and only send to webview if plugin didn't handle it". Sending the event to two targets looks weird to me. But again, I'm probably not the right person to look at this.
> 
> Unfortunately this is not possible right now. Currently ScrollableArea::handleGestureEvent() just swallows all events.
> 
> There's a TODO in EventHandler::handleGestureEvent() to add proper hit testing. I think once this is done it'll be possible to go back and do this.

I agree with Nico.  Dispatching the event to both targets seems kind of problematic.  I also wonder why the EventHandler wouldn't just forward the event to the plugin if that is what it hits.

[Sailesh Agrawal]

Comment on attachment 104578 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=104578&action=review

>>>> Source/WebKit/chromium/src/WebViewImpl.cpp:615
>>>> +
>>> 
>>> I'm not an expert in plugins, but I would've expected the flow to go "if plugin has touch focus, send to plugin, and only send to webview if plugin didn't handle it". Sending the event to two targets looks weird to me. But again, I'm probably not the right person to look at this.
>> 
>> Unfortunately this is not possible right now. Currently ScrollableArea::handleGestureEvent() just swallows all events.
>> 
>> There's a TODO in EventHandler::handleGestureEvent() to add proper hit testing. I think once this is done it'll be possible to go back and do this.
> 
> I agree with Nico.  Dispatching the event to both targets seems kind of problematic.  I also wonder why the EventHandler wouldn't just forward the event to the plugin if that is what it hits.

The EventHandler doesn't do hit testing so it can't tell if the gesture was for the plugin or not.

Safari does it the same way too so I think we can live with this for now.

[John Abd-El-Malek]

btw this looks good to me

[Darin Fisher (:fishd, Google)]

Comment on attachment 104578 [details]
Patch

OK, is there a bug about cleaning this up?

[Sailesh Agrawal]

(In reply to comment #9)
> (From update of attachment 104578 [details])
> OK, is there a bug about cleaning this up?

I couldn't find one so I filed this bug:
https://bugs.webkit.org/show_bug.cgi?id=67338

[WebKit Review Bot]

Comment on attachment 104578 [details]
Patch

Clearing flags on attachment: 104578

Committed r94261: <http://trac.webkit.org/changeset/94261>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.