Allow Blink to control Widget-level mouse capture in browser process |
||||||||
Issue descriptionMouse capture for OOPIFs was added in https://codereview.chromium.org/2339503002/, but in comments it was noted that this doesn't respect an event handler in a frame calling preventDefault on a MouseDown event, which should cancel the capture. Blink needs a mechanism to notify the browser process whether a frame wants to capture mouse events or not.
,
Sep 15 2016
,
Sep 15 2016
More important than matching our existing broken mouse capture behavior (issue 269917) is probably that we're about to ship PointerEvents and the setPointerCapture API. So what frame a mouse is captured to should really be up to blink / JavaScript anyway.
,
Sep 15 2016
The clarify the last comment: the browser should still be the ultimate decider here, isn't it? E.g. a setPointerCapture call from a cross-origin iframe should be effective only within the extent of the iframe in some cases to prevent malicious use, right? A related thought: the "allow-pointer-lock" sandbox token is defined to be for Pointer Lock API only. May be we should extend the definition to include PointerEvents.
,
Sep 15 2016
At the moment there is no restriction (already the case in Edge) and the security team convinced me it wasn't urgent. Issue 606896. But yeah, as part of issue 606896 the browser may want to enforce some restrictions (eg. around sandboxed iframes).
,
Sep 26 2016
Bug 650348 provides a test case that illustrates differing behavior between OOPIF and non-OOPIF cases. The problem is that right now the OOPIF needs to get the MouseUp so that it knows, for instance, that the user isn't dragging a selection or moving the scrollbar anymore. But in non-OOPIF the MouseUp is targeted to the frame under the cursor, and those things happen by side effect. I think that is a special case of this bug.
,
Oct 7 2016
Could it be that this code is now causing Issue 653860 ? This is a case where the iframe is from the same origin, though.
,
Oct 7 2016
#7: I don't think that is likely. The only landed changes for this are related to browser process event routing, which doesn't appear to be related.
,
Dec 14 2016
lfg@ pointed out that issue 671445 is likely a fallout from this bug.
,
Jun 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04 commit 94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04 Author: Ken Buchanan <kenrb@chromium.org> Date: Fri Jun 22 19:56:24 2018 Make Blink control browser mouse capture for scrollbar drags The current behavior to allow mouse input in the browser process to be locked to a given RenderWidgetHost is to send all mouse input between a MouseDown and a MouseUp to the same target. This is not in accordance with spec, and causes some site breakage. Whether a MouseDown should cause capture depends on the node that the event hits. Drag-highlighting text or clicking the thumb part of a scrollbar require capture, for instance, while most elements do not. This change removes the implicit capture currently done in the browser, and replaces it in the case of scrollbars with an explicit signal from Blink to start capturing. Other cases requiring capture still need to be implemented. Bug: 647378 , 849862 Change-Id: Ifc4b690c36927ed48edd2604d40742e130295dcf Reviewed-on: https://chromium-review.googlesource.com/1099183 Commit-Queue: Ken Buchanan <kenrb@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Ella Ge <eirage@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org> Cr-Commit-Position: refs/heads/master@{#569740} [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/content/browser/renderer_host/input/input_router_client.h [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/content/browser/renderer_host/input/input_router_impl.cc [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/content/browser/renderer_host/input/input_router_impl.h [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/content/browser/renderer_host/input/input_router_impl_unittest.cc [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/content/browser/renderer_host/input/mock_input_router_client.h [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/content/browser/renderer_host/render_widget_host_impl.cc [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/content/browser/renderer_host/render_widget_host_impl.h [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/content/browser/renderer_host/render_widget_host_input_event_router.cc [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/content/browser/renderer_host/render_widget_host_input_event_router.h [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/content/browser/site_per_process_hit_test_browsertest.cc [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/content/common/input/input_handler.mojom [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/content/renderer/render_frame_impl.cc [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/content/renderer/render_frame_impl.h [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/content/renderer/render_widget.cc [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/content/renderer/render_widget.h [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/content/renderer/render_widget_unittest.cc [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/third_party/blink/public/web/web_local_frame_client.h [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/third_party/blink/renderer/core/exported/local_frame_client_impl.cc [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/third_party/blink/renderer/core/exported/local_frame_client_impl.h [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/third_party/blink/renderer/core/frame/local_frame_client.h [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/third_party/blink/renderer/core/input/event_handler.cc [modify] https://crrev.com/94c0beb64439fec7c5e5b0d9b7d45f2bcab4dc04/third_party/blink/renderer/core/input/event_handler.h
,
Jun 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5aa4e3d02d9b674032174732e5998afa924c0523 commit 5aa4e3d02d9b674032174732e5998afa924c0523 Author: Charlie Reis <creis@chromium.org> Date: Tue Jun 26 18:12:19 2018 Merge to M68: Make Blink control browser mouse capture for scrollbar drags The current behavior to allow mouse input in the browser process to be locked to a given RenderWidgetHost is to send all mouse input between a MouseDown and a MouseUp to the same target. This is not in accordance with spec, and causes some site breakage. Whether a MouseDown should cause capture depends on the node that the event hits. Drag-highlighting text or clicking the thumb part of a scrollbar require capture, for instance, while most elements do not. This change removes the implicit capture currently done in the browser, and replaces it in the case of scrollbars with an explicit signal from Blink to start capturing. Other cases requiring capture still need to be implemented. Bug: 647378 , 849862 Change-Id: Ifc4b690c36927ed48edd2604d40742e130295dcf Reviewed-on: https://chromium-review.googlesource.com/1099183 Commit-Queue: Ken Buchanan <kenrb@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Ella Ge <eirage@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#569740} Reviewed-on: https://chromium-review.googlesource.com/1112757 Reviewed-by: Ken Buchanan <kenrb@chromium.org> Reviewed-by: Charlie Reis <creis@chromium.org> Cr-Commit-Position: refs/branch-heads/3440@{#535} Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733} [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/content/browser/renderer_host/input/input_router_client.h [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/content/browser/renderer_host/input/input_router_impl.cc [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/content/browser/renderer_host/input/input_router_impl.h [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/content/browser/renderer_host/input/input_router_impl_unittest.cc [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/content/browser/renderer_host/input/mock_input_router_client.h [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/content/browser/renderer_host/render_widget_host_impl.cc [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/content/browser/renderer_host/render_widget_host_impl.h [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/content/browser/renderer_host/render_widget_host_input_event_router.cc [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/content/browser/renderer_host/render_widget_host_input_event_router.h [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/content/browser/site_per_process_hit_test_browsertest.cc [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/content/common/input/input_handler.mojom [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/content/renderer/render_frame_impl.cc [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/content/renderer/render_frame_impl.h [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/content/renderer/render_widget.cc [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/content/renderer/render_widget.h [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/content/renderer/render_widget_unittest.cc [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/third_party/blink/public/web/web_frame_client.h [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/third_party/blink/renderer/core/exported/local_frame_client_impl.cc [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/third_party/blink/renderer/core/exported/local_frame_client_impl.h [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/third_party/blink/renderer/core/frame/local_frame_client.h [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/third_party/blink/renderer/core/input/event_handler.cc [modify] https://crrev.com/5aa4e3d02d9b674032174732e5998afa924c0523/third_party/blink/renderer/core/input/event_handler.h
,
Aug 21
Issue 875839 has been merged into this issue.
,
Aug 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/334029e1be165c014c310d2f371433eebc0ee7ff commit 334029e1be165c014c310d2f371433eebc0ee7ff Author: Ken Buchanan <kenrb@chromium.org> Date: Fri Aug 31 19:29:00 2018 Capture mouse events to a widget during drag selection Currently when the mouse cursor moves between OOPIF boundaries while a selection is being dragged, the frame containing the selection stops receiving mouse events. This causes a number of bugs such as the relevant frame not being aware of when the mouse button is released. This CL causes Blink's SelectionController to signal the browser process for mouse capture to the current RenderWidgetHost. Bug: 876662 , 864957 , 647378 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Change-Id: Id9d4bb6bdb6a3fa33b4e15af35a0588e952755fe Reviewed-on: https://chromium-review.googlesource.com/1197409 Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Commit-Queue: Ken Buchanan <kenrb@chromium.org> Cr-Commit-Position: refs/heads/master@{#588115} [modify] https://crrev.com/334029e1be165c014c310d2f371433eebc0ee7ff/content/browser/site_per_process_hit_test_browsertest.cc [modify] https://crrev.com/334029e1be165c014c310d2f371433eebc0ee7ff/third_party/blink/renderer/core/editing/selection_controller.cc
,
Aug 31
This is partially there. The original mouse capture mechanism had to be (mostly) removed earlier this year, and Blink now signals for the browser to capture mouse input to a specific RenderWidgetHostView in the following scenarios: - mouse is dragging a scrollbar thumb - mouse is highlighting text via a drag I think there is more here because I can see other places in Blink where mouse input could potentially be locked to a frame (e.g. MouseEventManager::SetCapturesDragging()), but right now I don't know what use cases activate that and whether it really represents a functional bug.
,
Sep 3
If there are situations where mouse down can be reported to JavaScript, but there is no way to guarantee that the code will be notified of the corresponding mouse up event, then yes, this is a functional bug. As you have seen - it is impossible to properly implement scrollbar dragging or text highlighting if you cannot be sure whether the mouse is still down. JavaScript face the very same problem: If they don't receive mouse up events, a lot of user interfaces can get into broken states and there is no way for the devs to workaround this in an acceptable manner. At the very least, a pointer lock API needs to be provided so that for crucial gestures it is guaranteed that the code will be notified if either the mouse button was released or that pointer lock was lost and it is likely that the mouse button state is unknown from now on. If there was a reliable API that provided the global mouse button state, then all of this would not be as critical as it is, but as long as mouse-up is not guaranteed to be triggered, and there is no other way to register for such an event, I would definitely say this is a functional bug and not just a missing feature.
,
Sep 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/71492bd2eba9f53ef41d54ac93aa7c7121928d6b commit 71492bd2eba9f53ef41d54ac93aa7c7121928d6b Author: Ken Buchanan <kenrb@chromium.org> Date: Mon Sep 10 17:46:33 2018 Capture mouse events to a widget during drag selection Currently when the mouse cursor moves between OOPIF boundaries while a selection is being dragged, the frame containing the selection stops receiving mouse events. This causes a number of bugs such as the relevant frame not being aware of when the mouse button is released. This CL causes Blink's SelectionController to signal the browser process for mouse capture to the current RenderWidgetHost. Bug: 876662 , 864957 , 647378 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Change-Id: Id9d4bb6bdb6a3fa33b4e15af35a0588e952755fe Reviewed-on: https://chromium-review.googlesource.com/1197409 Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Commit-Queue: Ken Buchanan <kenrb@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#588115}(cherry picked from commit 334029e1be165c014c310d2f371433eebc0ee7ff) Reviewed-on: https://chromium-review.googlesource.com/1216715 Reviewed-by: Ken Buchanan <kenrb@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#226} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/71492bd2eba9f53ef41d54ac93aa7c7121928d6b/content/browser/site_per_process_hit_test_browsertest.cc [modify] https://crrev.com/71492bd2eba9f53ef41d54ac93aa7c7121928d6b/third_party/blink/renderer/core/editing/selection_controller.cc
,
Oct 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3561c3c6f73f447b803ef49b8c2f86c181e7edb7 commit 3561c3c6f73f447b803ef49b8c2f86c181e7edb7 Author: Ken Buchanan <kenrb@chromium.org> Date: Fri Oct 12 23:23:50 2018 Make subframe widgets implicitly capture mouse events on MouseDown Frame-level capture of mouse events implicitly happens when a MouseDown is targeted to an element in a subframe. Currently this does not work across frames in different processes because the browser process does not receive a notification about capture being active. This patch adds that notification in order to initiate widget-level capture in that case. Bug: 888342 , 647378 Change-Id: I26f55b2690e9ecfbd0243a9a610541abdff3b575 Reviewed-on: https://chromium-review.googlesource.com/c/1271588 Commit-Queue: Ken Buchanan <kenrb@chromium.org> Reviewed-by: Dave Tapuska <dtapuska@chromium.org> Cr-Commit-Position: refs/heads/master@{#599408} [modify] https://crrev.com/3561c3c6f73f447b803ef49b8c2f86c181e7edb7/content/browser/site_per_process_hit_test_browsertest.cc [modify] https://crrev.com/3561c3c6f73f447b803ef49b8c2f86c181e7edb7/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process [modify] https://crrev.com/3561c3c6f73f447b803ef49b8c2f86c181e7edb7/third_party/blink/renderer/core/input/event_handler.cc [modify] https://crrev.com/3561c3c6f73f447b803ef49b8c2f86c181e7edb7/third_party/blink/renderer/core/input/event_handler.h
,
Oct 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fd61bc6783b1a3a18db49f860cbe1b92fa25475b commit fd61bc6783b1a3a18db49f860cbe1b92fa25475b Author: Ken Buchanan <kenrb@chromium.org> Date: Fri Oct 19 19:35:32 2018 Make subframe widgets implicitly capture mouse events on MouseDown Frame-level capture of mouse events implicitly happens when a MouseDown is targeted to an element in a subframe. Currently this does not work across frames in different processes because the browser process does not receive a notification about capture being active. This patch adds that notification in order to initiate widget-level capture in that case. Bug: 888342 , 647378 Change-Id: I26f55b2690e9ecfbd0243a9a610541abdff3b575 Reviewed-on: https://chromium-review.googlesource.com/c/1271588 Commit-Queue: Ken Buchanan <kenrb@chromium.org> Reviewed-by: Dave Tapuska <dtapuska@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#599408}(cherry picked from commit 3561c3c6f73f447b803ef49b8c2f86c181e7edb7) Reviewed-on: https://chromium-review.googlesource.com/c/1292272 Reviewed-by: Ken Buchanan <kenrb@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#164} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/fd61bc6783b1a3a18db49f860cbe1b92fa25475b/content/browser/site_per_process_hit_test_browsertest.cc [modify] https://crrev.com/fd61bc6783b1a3a18db49f860cbe1b92fa25475b/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process [modify] https://crrev.com/fd61bc6783b1a3a18db49f860cbe1b92fa25475b/third_party/blink/renderer/core/input/event_handler.cc [modify] https://crrev.com/fd61bc6783b1a3a18db49f860cbe1b92fa25475b/third_party/blink/renderer/core/input/event_handler.h
,
Oct 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fd61bc6783b1a3a18db49f860cbe1b92fa25475b Commit: fd61bc6783b1a3a18db49f860cbe1b92fa25475b Author: kenrb@chromium.org Commiter: kenrb@chromium.org Date: 2018-10-19 19:35:32 +0000 UTC Make subframe widgets implicitly capture mouse events on MouseDown Frame-level capture of mouse events implicitly happens when a MouseDown is targeted to an element in a subframe. Currently this does not work across frames in different processes because the browser process does not receive a notification about capture being active. This patch adds that notification in order to initiate widget-level capture in that case. Bug: 888342 , 647378 Change-Id: I26f55b2690e9ecfbd0243a9a610541abdff3b575 Reviewed-on: https://chromium-review.googlesource.com/c/1271588 Commit-Queue: Ken Buchanan <kenrb@chromium.org> Reviewed-by: Dave Tapuska <dtapuska@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#599408}(cherry picked from commit 3561c3c6f73f447b803ef49b8c2f86c181e7edb7) Reviewed-on: https://chromium-review.googlesource.com/c/1292272 Reviewed-by: Ken Buchanan <kenrb@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#164} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Oct 31
I'm not aware of any more work to be done on this, so closing it out. Any further problems around mouse capture and OOPIFs can be tracked as independent bugs.
,
Nov 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c0558c5bdbd9676fe8bbcdd2cdd524c87b5e475b commit c0558c5bdbd9676fe8bbcdd2cdd524c87b5e475b Author: Ella Ge <eirage@chromium.org> Date: Thu Nov 08 19:57:54 2018 remove set oopif mouse capture on text selection In crrev.com/588115 introduced capture mouse events to widget during text selection; and a later change crrev.com/599408 make subframe widgets capture mouse events on MouseDown, which follows the behavior without OOPIF. Since we are captureing mouse events on MouseDown and release the capture on MouseUp, we don't need to treat the text selection case special. Bug: 647378 Change-Id: I207777d16e11e069fb1ab60579d45324cb5b06c4 Reviewed-on: https://chromium-review.googlesource.com/c/1308606 Reviewed-by: Ken Buchanan <kenrb@chromium.org> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Mustaq Ahmed <mustaq@chromium.org> Commit-Queue: Ella Ge <eirage@chromium.org> Cr-Commit-Position: refs/heads/master@{#606573} [modify] https://crrev.com/c0558c5bdbd9676fe8bbcdd2cdd524c87b5e475b/content/browser/site_per_process_hit_test_browsertest.cc [modify] https://crrev.com/c0558c5bdbd9676fe8bbcdd2cdd524c87b5e475b/third_party/blink/renderer/core/editing/selection_controller.cc |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 Deleted