Investigate pinch gestures for OOPIFs |
|||||
Issue descriptionIt looks like pinch isn't implemented correctly when routing events between multiple processes rendering a single page. Pinch zoom does appear to work correctly on BrowserPlugin which uses input routing for gesture events, although we aren't clear why. Pinch gestures should always go to top-level frames to trigger page zoom. Accordingly, top-level RenderWidgetHostViews should send them directly to their renderers rather than trying to pass them through RenderWidgetHostInputEventRouter.
,
Jul 18 2016
Relevant comment by bokan@ from https://codereview.chromium.org/2034213002/, outlining a problem that needs to be fixed: "... [T]oday we send the Pinch gesture to Blink, it synthesizes a wheel event, and if that's unhandled we pinch-zoom. This breaks for OOPIF (and is probably already broken today in Mac with --enable-site-isolation). It works outside site-isolation because the WebViewImpl handles the pinch-zoom so it can apply it globally. If this is to work with OOPIF we need to move this logic up into the browser. So the browser should synthesize a wheel event, send it to the renderer associated with the frame, if the ACK comes back unhandled, send the pinch-zoom event to the root view."
,
Jul 18 2016
We can keep one bug open fo this, but I think the work to fix this for touch-screen should probably land separately from the work for touch-pad. I'll work on the touch-screen CL first unless there are any reasons to do the touch-pad work first.
,
Jul 18 2016
If I recall correctly, the logic mentioned in c#2 was actually once in the browser, but later moved to the renderer. Please take a look at CL landed for issue 451559 .
,
Jul 18 2016
No, that's making the touchpad activate pinch-zoom rather than browser zoom (i.e. ctrl+/-). Prior to that, there was no "touchpad" pinch zoom to speak of.
,
Jul 18 2016
Considering we are shipping first to desktop platforms, shouldn't we be doing touchpad support first and touchscreen support later?
,
Jul 18 2016
FWIW, touchscreens do exist on desktop (laptops mostly).
,
Jul 18 2016
FWIW, I expect the work for touchscreen to be quite small by comparison, so it should be out of the way quickly, then we'll get to the touchpad shortly afterwards.
,
Jul 18 2016
#c7, I'm aware, but the percentage is likely small. #c8 - sgtm
,
Jul 29 2016
,
Aug 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/237a3cdd338750a6a35d336912c5830868380272 commit 237a3cdd338750a6a35d336912c5830868380272 Author: wjmaclean <wjmaclean@chromium.org> Date: Fri Aug 05 22:35:13 2016 Re-route Touchscreen GesturePinch events to root view. This CL fixes the routing of GesturePinch* events so that they always go to the root view. Also, any other gesture events between GesturePinchBegin and GesturePinchEnd (namely GestureScrollUpdate) are also routed to the root view, so that the visual viewport pans correctly. BUG= 627926 Review-Url: https://codereview.chromium.org/2186983002 Cr-Commit-Position: refs/heads/master@{#410198} [modify] https://crrev.com/237a3cdd338750a6a35d336912c5830868380272/content/browser/renderer_host/render_widget_host_impl.h [modify] https://crrev.com/237a3cdd338750a6a35d336912c5830868380272/content/browser/renderer_host/render_widget_host_input_event_router.cc [modify] https://crrev.com/237a3cdd338750a6a35d336912c5830868380272/content/browser/renderer_host/render_widget_host_input_event_router.h [modify] https://crrev.com/237a3cdd338750a6a35d336912c5830868380272/content/browser/site_per_process_browsertest.cc
,
Aug 8 2016
This is now fixed for touchscreen pinch gestures.
,
Mar 13 2017
With https://codereview.chromium.org/2745783002/ this is also workig for TouchPad gestures (Mac) for both Non-OOPIF WebView, and OOPIF WebView. I think this is working correctly for everything except propagation of page scale factor, which is being tracked in its own bug, so closing. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by wjmaclean@chromium.org
, Jul 13 2016Status: Assigned (was: Available)