New issue
Advanced search Search tips

Issue 855840 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 13
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Compat



Sign in to add a comment

iframe scroll events do not propagate to parent

Reported by ross...@gmail.com, Jun 23 2018

Issue description

UserAgent: Mozilla/5.0 (X11; CrOS x86_64 10575.55.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.87 Safari/537.36
Platform: 10575.55.0 (Official Build) stable-channel lulu

Example URL:
http://www.scp-wiki.net/scp-3211

Steps to reproduce the problem:
1. Create a page that is able to scroll containing an iframe that is not able to scroll
2. Position cursor over iframe
3. Attempt to scroll

What is the expected behavior?
Scroll event propagates from iframe to parent

What went wrong?
Scroll event does not propagate and no scrolling occurs

Does it occur on multiple sites: N/A

Is it a problem with a plugin? No 

Did this work before? Yes Unsure

Does this work in other browsers? N/A

Chrome version: 67.0.3396.87  Channel: stable
OS Version: 10575.55.0
Flash Version:
 
Components: Blink>Scroll
Cc: kenrb@chromium.org bokan@chromium.org mcnee@chromium.org creis@chromium.org alex...@chromium.org
Components: Internals>Sandbox>SiteIsolation
Status: Untriaged (was: Unconfirmed)
Thanks for the report!  Can you clarify where you're seeing the problem, since there are several iframes on the repro page?

I think I can repro on a Pixel 1 with 67.0.3396.87 if I put the cursor over the "languages" pane (which is a cross-site iframe to scpnet.org) and do a two-finger scroll.  The first time I try scrolling, it doesn't work.  The second time it usually works though.  Setting chrome://flags/#site-isolation-trial-opt-out to "Opt out" seems to fix this, so I'm adding the site isolation label.  I also can't repro on a recent Mac canary, so we might've already fixed this, though it could also be a CrOS-specific issue.

Adding a few folks knowledgeable about scroll and scroll bubbling to triage this further.

Comment 3 by kenrb@chromium.org, Jun 25 2018

Cc: sahel@chromium.org

Comment 4 by creis@chromium.org, Jun 25 2018

Cc: wjmaclean@chromium.org
Labels: -Pri-2 Pri-1
Owner: sahel@chromium.org
Status: Assigned (was: Untriaged)
Sounds like  issue 730103 .  sahel@, that one was fixed by scroll latching.  Can you take a look or help find the right owner?

I can verify on ChromeOS 67.0.3396.87 using the touchpad but not the mouse.  No repro on Windows or Mac using mouse, but that's just testing on desktop.

Comment 5 by mcnee@chromium.org, Jun 25 2018

No repro on stable, beta, or dev for linux touchpad scrolling.

Comment 6 by ross...@gmail.com, Jun 25 2018

Re: Comment 2:

Should have been clearer in the initial report.

This behaviour is happening on all iframes - jsfiddle for example embeds its output content into an iframe. I can reproduce here https://jsfiddle.net/u8tbjdr3/

On the linked page, having zoomed in slightly such that scrolling is available, and aligning my cursor over the yellow div:
- I cannot reliably scroll using touchpad - the scroll may take effect a small amount, often on the second scroll, but most of the time no scrolling occurs at all
- I cannot pinch-to-zoom on the touchpad
- I cannot scroll using a mousewheel (same behaviour as touchpad)
- Zooming using Ctrl+mousewheel seems fine
- Setting chrome://flags/#site-isolation-trial-opt-out to "Opt out" does fix the issue completely

Moving the mouse off the iframe and attempting to scroll works as expected

I've been seeing this behaviour on all iframe elements in all sites (eg embedded videos, ads, etc)

I know of at least one person who's having this issue on Chrome on Win10, though I don't have more detailed info for them.

Comment 7 by mcnee@chromium.org, Jun 25 2018

I can repro on http://csreis.github.io/tests/cross-site-iframe.html on chrome os 69.0.3464.0. Scroll bubbling does not happen for the first scroll gesture, but does for subsequent scroll gestures.

Comment 8 by mcnee@chromium.org, Jun 25 2018

Re c#6: Touchpad pinch not working over a cross-site iframe is a known issue and is fixed as of 69.0.3450.0 (  crbug.com/739213#c7  ).

Comment 9 by sahel@chromium.org, Jun 27 2018

I can reproduce the issue using steps in comment #6 on ChromeOS(both with touchpad and mouse wheel), but not on Linux.

Still debugging to find the root cause of the issue.
We're hitting the early return due to the check for |target_view == touchpad_gesture_target_.target| in RenderWidgetHostInputEventRouter::BubbleScrollEvent. The touchpad gesture target is set due to the kGestureFlingStart from the previous gesture and is never cleared before we try to bubble.
the GSU events get bubbled instead of being targeted, it is strange that we are targeting the GestureFlingStart event.
Owner: mcnee@chromium.org
It seems unfortunate to have these touchpad fling events represented as gesture events rather than mouse wheel events. In any case, it looks like the fling controller uses the gesture event to produce mouse wheels, so it seems reasonable to treat these gesture events as though they were mouse wheel events for the purpose of RWHV targeting.

I can take this.
>it looks like the fling controller uses the gesture event to produce mouse wheels, so it seems reasonable to treat these gesture events as though they were mouse wheel events for the purpose of RWHV targeting

I agree, it seems like targeting the GFS event with touchpad source to the wheel target is the right approach. With this approach we should do the same thing for GestureFlingCancel events that are generated and send before creating webMouseWheelEvents:
https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_view_event_handler.cc?rcl=1874b89ff28b7042cff4064956f0145aac723738&l=438

It might be trickier for GFCs to be routed to the wheel target since they are routed before routing wheel events and at that point we might not have the wheel target.

Another solution can be not setting the touchpad_gesture target on GFS and GFC events, I don't know if this approach causes any regressions or not.

>I can take this.
Thanks mcnee@ for taking the ownership of the bug since I am working on a few other scroll related p1 bugs.
Status: Started (was: Assigned)
CLs: https://chromium-review.googlesource.com/c/chromium/src/+/1138637/ and https://chromium-review.googlesource.com/c/chromium/src/+/1126454/

I've also found a related bug involving the use of both a touchscreen and touchpad which I've filed a separate bug for: issue 865151
Project Member

Comment 16 by bugdroid1@chromium.org, Jul 19

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

commit 33bd48543efb26beb7244c60cfde3dfd6306fe92
Author: Kevin McNee <mcnee@chromium.org>
Date: Thu Jul 19 14:36:46 2018

Only generate a synthetic fling cancel at the start of a touchpad scroll

As part of  issue 855840 , we will need to hit test touchpad gesture fling
cancel events. However, we currently generate a synthetic fling cancel
before every wheel event in a scroll. This is excessive, since we have
wheel scroll latching, and would become expensive, since we would need
to hit test them all.

We now only generate a synthetic fling cancel at the beginning of a
touchpad scroll.

Bug:  855840 
Change-Id: I108393814065c90cea80ba5326772a1764526ad7
Reviewed-on: https://chromium-review.googlesource.com/1138637
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Reviewed-by: Sahel Sharify <sahel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576486}
[modify] https://crrev.com/33bd48543efb26beb7244c60cfde3dfd6306fe92/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/33bd48543efb26beb7244c60cfde3dfd6306fe92/content/browser/renderer_host/render_widget_host_view_event_handler.cc
[modify] https://crrev.com/33bd48543efb26beb7244c60cfde3dfd6306fe92/ui/events/blink/web_input_event.cc
[modify] https://crrev.com/33bd48543efb26beb7244c60cfde3dfd6306fe92/ui/events/blink/web_input_event.h

Project Member

Comment 17 by bugdroid1@chromium.org, Jul 30

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

commit 6585d96a632da78c0c285e146d02405f748e876f
Author: Kevin McNee <mcnee@chromium.org>
Date: Mon Jul 30 16:44:28 2018

Fix stale touchpad gesture target which prevented scroll bubbling

In RenderWidgetHostInputEventRouter::DispatchTouchpadGestureEvent,
we set |touchpad_gesture_target_| for kGesturePinchBegin and
kGestureFlingStart events, but don't clear it until we receive
another such event. So if we perform such a gesture over a parent
RenderWidgetHostView and then scroll a child frame, scroll bubbling
will be prevented because the RenderWidgetHostInputEventRouter
believes that the parent has unrelated gesture events in progress.

Now when we set |touchpad_gesture_target_| from a pinch begin, we
clear it when we receive the pinch end.

For touchpad gesture flings, they should really be associated with
the mouse wheel events of the user's gesture, so we won't target them
separately and instead just use the existing target for the mouse
wheels.

We also correct the targeting of fling cancel events so that they
are sent to the same target that received the fling start (if it
exists).

Bug:  855840 
Change-Id: Ie5b2e35dd8f50a98d503fad26f6897cec834b01f
Reviewed-on: https://chromium-review.googlesource.com/1126454
Reviewed-by: Sahel Sharify <sahel@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579066}
[modify] https://crrev.com/6585d96a632da78c0c285e146d02405f748e876f/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/6585d96a632da78c0c285e146d02405f748e876f/content/browser/renderer_host/render_widget_host_input_event_router.h
[modify] https://crrev.com/6585d96a632da78c0c285e146d02405f748e876f/content/browser/site_per_process_hit_test_browsertest.cc
[modify] https://crrev.com/6585d96a632da78c0c285e146d02405f748e876f/content/browser/site_per_process_mac_browsertest.mm

Labels: Merge-Request-69
Requesting merge of https://chromium.googlesource.com/chromium/src.git/+/6585d96a632da78c0c285e146d02405f748e876f to M69. The CL it depended on landed in time, but the fix itself didn't.

Impact: If a user scrolls a parent frame with a touchpad on ChromeOS, they may be unable to subsequently scroll if the cursor is moved over a cross-site iframe.
Project Member

Comment 19 by sheriffbot@chromium.org, Aug 2

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: M-69
Labels: -Merge-Review-69 Merge-Approved-69
Merge approved, M69.
Project Member

Comment 22 by bugdroid1@chromium.org, Aug 13

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f8740b12eb1635805171d1285471f30e937eb052

commit f8740b12eb1635805171d1285471f30e937eb052
Author: Kevin McNee <mcnee@chromium.org>
Date: Mon Aug 13 16:18:00 2018

Merge M69: Fix stale touchpad gesture target which prevented scroll bubbling

TBR=kenrb@chromium.org

Fix stale touchpad gesture target which prevented scroll bubbling

In RenderWidgetHostInputEventRouter::DispatchTouchpadGestureEvent,
we set |touchpad_gesture_target_| for kGesturePinchBegin and
kGestureFlingStart events, but don't clear it until we receive
another such event. So if we perform such a gesture over a parent
RenderWidgetHostView and then scroll a child frame, scroll bubbling
will be prevented because the RenderWidgetHostInputEventRouter
believes that the parent has unrelated gesture events in progress.

Now when we set |touchpad_gesture_target_| from a pinch begin, we
clear it when we receive the pinch end.

For touchpad gesture flings, they should really be associated with
the mouse wheel events of the user's gesture, so we won't target them
separately and instead just use the existing target for the mouse
wheels.

We also correct the targeting of fling cancel events so that they
are sent to the same target that received the fling start (if it
exists).

(cherry picked from commit 6585d96a632da78c0c285e146d02405f748e876f)

Bug:  855840 
Change-Id: Ie5b2e35dd8f50a98d503fad26f6897cec834b01f
Reviewed-on: https://chromium-review.googlesource.com/1126454
Reviewed-by: Sahel Sharify <sahel@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#579066}
Reviewed-on: https://chromium-review.googlesource.com/1172793
Reviewed-by: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#563}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/f8740b12eb1635805171d1285471f30e937eb052/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/f8740b12eb1635805171d1285471f30e937eb052/content/browser/renderer_host/render_widget_host_input_event_router.h
[modify] https://crrev.com/f8740b12eb1635805171d1285471f30e937eb052/content/browser/site_per_process_hit_test_browsertest.cc
[modify] https://crrev.com/f8740b12eb1635805171d1285471f30e937eb052/content/browser/site_per_process_mac_browsertest.mm

Status: Fixed (was: Started)

Sign in to add a comment