New issue
Advanced search Search tips

Issue 841270 link

Starred by 2 users
Status: Fixed
Owner:
xidac...@chromium.org
Closed: Jun 2018
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

allowed_touch_action_ is wrong when a gesture event is bubbled from an iframe to main frame

Project Member Reported by xidac...@chromium.org, May 9 2018 
Let's use a simple example of a page that has one iframe, and a gesture starts touching the iframe and scrolls through the iframe boundary.

Without OOPIF, the allowed_touch_action_ will always be the one from the iframe even if the gesture event bubbles to the main frame.

With OOPIF, when the gesture event bubbles to the main frame, the allowed_touch_action_ becomes the default value which is auto.

I think the right behavior should be matching the DOM bubbling case.

Comment 1 by dtapu...@chromium.org, May 9 2018 

Specifically due to bubbling of the gesture event and the computation of the touch_action on the touchstart. The parent frame never sees the touchstart so it never has a chance to compute the touch-action.

We might want to separate the computation of the touch-action in one IPC and and the dispatch of the touch-start.
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 19 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/064bdb858e0ef62ba52bf6d4b3029329666142aa

commit 064bdb858e0ef62ba52bf6d4b3029329666142aa
Author: Xida Chen <xidachen@chromium.org>
Date: Tue Jun 19 01:26:52 2018

Remove WebGestureEvent::is_bubbled_from_child_frame

This CL removes that attributes. It also adds a gesture scroll bubbling
case to ensure that the behavior is correct.

Due to our previous temporary fix, we set |scrolling_touch_action_| to
Auto at GSB if it has no value. To prove that this CL is actually working,
we do:
PS#1: remove the set |scrolling_touch_action_| to Auto at GSB, and
the test will crash.
PS#2: Keep 1, add a line in CrossProcessFrameConnector::BubbleScrollEvent
which sets the |scrolling_touch_action_| to Auto. The test will pass.
PS#3: Keep the temporary fix which sets |scrolling_touch_action_| to
Auto at GSB.

TBR=pdr@chromium.org

Bug:  852835 ,  841270 
Change-Id: Ib33a39b4b0ef61e88645da427b920687bbcc80c9
Reviewed-on: https://chromium-review.googlesource.com/1101315
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568281}
[modify] https://crrev.com/064bdb858e0ef62ba52bf6d4b3029329666142aa/content/browser/frame_host/cross_process_frame_connector.cc
[modify] https://crrev.com/064bdb858e0ef62ba52bf6d4b3029329666142aa/content/browser/renderer_host/input/input_router.h
[modify] https://crrev.com/064bdb858e0ef62ba52bf6d4b3029329666142aa/content/browser/renderer_host/input/input_router_impl.cc
[modify] https://crrev.com/064bdb858e0ef62ba52bf6d4b3029329666142aa/content/browser/renderer_host/input/input_router_impl.h
[modify] https://crrev.com/064bdb858e0ef62ba52bf6d4b3029329666142aa/content/browser/renderer_host/input/touch_action_filter.cc
[modify] https://crrev.com/064bdb858e0ef62ba52bf6d4b3029329666142aa/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/064bdb858e0ef62ba52bf6d4b3029329666142aa/content/browser/site_per_process_hit_test_browsertest.cc
[add] https://crrev.com/064bdb858e0ef62ba52bf6d4b3029329666142aa/content/test/data/frame_tree/page_with_pany_frame.html
[modify] https://crrev.com/064bdb858e0ef62ba52bf6d4b3029329666142aa/third_party/blink/public/platform/web_gesture_event.h

Comment 3 by xidac...@chromium.org, Jun 19 2018

Status: Fixed (was: Assigned)
►

Sign in to add a comment