iframe scroll events do not propagate to parent
Reported by
ross...@gmail.com,
Jun 23 2018
|
||||||||||||
Issue descriptionUserAgent: 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:
,
Jun 25 2018
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.
,
Jun 25 2018
,
Jun 25 2018
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.
,
Jun 25 2018
No repro on stable, beta, or dev for linux touchpad scrolling.
,
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.
,
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.
,
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 ).
,
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.
,
Jul 3
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.
,
Jul 4
the GSU events get bubbled instead of being targeted, it is strange that we are targeting the GestureFlingStart event.
,
Jul 4
The GestureFlingStart is coming from here: https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_view_event_handler.cc?rcl=1874b89ff28b7042cff4064956f0145aac723738&l=462
,
Jul 4
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.
,
Jul 4
>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.
,
Jul 18
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
,
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
,
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
,
Aug 1
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.
,
Aug 2
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
,
Aug 9
,
Aug 9
Merge approved, M69.
,
Aug 13
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
,
Aug 13
|
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by dtapu...@chromium.org
, Jun 25 2018