| Summary: | Chromium Mac: PDF: Scrollsbars should be visible when scrolling using the two finger gesture on Lion | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Sailesh Agrawal <sail> | ||||
| Component: | New Bugs | Assignee: | Sailesh Agrawal <sail> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | fishd, jam, webkit.review.bot | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Sailesh Agrawal
2011-08-19 15:00:22 PDT
Created attachment 104578 [details]
Patch
This fixes the following chromium bug: http://code.google.com/p/chromium/issues/detail?id=92606 Ping! 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. 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. 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. 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. btw this looks good to me Comment on attachment 104578 [details]
Patch
OK, is there a bug about cleaning this up?
(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 Comment on attachment 104578 [details] Patch Clearing flags on attachment: 104578 Committed r94261: <http://trac.webkit.org/changeset/94261> All reviewed patches have been landed. Closing bug. |