This is also another issue caused by us having two paths for sending events I guess. When we move the node.cpp path (i.e. pointerlock path) to the normal path we should be able to have this resolved.

mustaq@ mentioned that avoiding direct dispatch through Node is tricky and complicate. I think fixing this within node.cc for now instead of merging the two path is probably easier for this single issue...

I don't think the expected outcome is clear here:
- The default action of back/forward is navigation.
- The PointerLock API doesn't say anything about overriding these default action right?

To me, this sounds like WAI.

These buttons should be forwarded now 

https://www.chromestatus.com/feature/5088301178421248

The browser still does the default action (=navigation) unless the page calls event.preventDefault() on mousedown event handler.

Which is what we do, however, it is ignored/doesn't work when the mouse is captured, and the page still navigates even though the event is received. -- whereas when it is not locked, it works as expected (not navigating). 


(Sorry if this is redundant, it seemed like my initial post may not have been clear.)

You are right, and we need to fix this.  Looks like I misread your initial description, my fault.

Hmmmm, I tested with chrome version 67.0.3396.87. It doesn't navigate when I preventDefault on mouseup with pointer locked.

Reporter: Could you please provide any test page to show the failure?

Since there hasn't been any feedback I'm closing this for now. Reporter, if you happen to get more information and a test page feel free to comment here or file a new bug with the request information.

This issue is certainly still present.
I made a simple test page. Press E to go fullscreen or F to lock the pointer, then press mouse buttons 4 and 5. You'll notice that the browser navigates back only when the pointer is locked, even though the mouseup event (the one that triggers the navigation) is cancelled on both the element in question and the document itself.

Deleted: index.html 2.2 KB

index.html 2.2 KB View Download

I see. Thanks for the test page.

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

commit c18ebe7a9d694b01b05ca77644518f99087fb379
Author: Ella Ge <eirage@chromium.org>
Date: Wed Jul 25 15:03:42 2018

Locked mouse event check ShouldGenerateAppCommand

When pointer is locked, we still need to check ShouldGenerateAppCommand
and set event handled so that won't generate unnecessary navigate
appcommand when kExtendedMouseButtons is enabled. This causes unable
to preventDefault on mouse back/forward buttons to prevent navigation
while pointer is locked.

Bug:  852709 
Change-Id: I5f75a68e36037210c38310f8d83825c5efdf311e
Reviewed-on: https://chromium-review.googlesource.com/1148886
Commit-Queue: Ella Ge <eirage@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577889}
[modify] https://crrev.com/c18ebe7a9d694b01b05ca77644518f99087fb379/content/browser/renderer_host/render_widget_host_view_event_handler.cc
[add] https://crrev.com/c18ebe7a9d694b01b05ca77644518f99087fb379/third_party/WebKit/LayoutTests/external/wpt/pointerlock/mouse_buttons_back_forward-manual.html
[add] https://crrev.com/c18ebe7a9d694b01b05ca77644518f99087fb379/third_party/WebKit/LayoutTests/external/wpt_automation/pointerlock/mouse_buttons_back_forward-manual-automation.js

 Issue 869860  has been merged into this issue.

Request merge #14 into M69

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

Per comment #0, this is regressed in M67 (not M69 regression). Rejecting merge to M69. Pls let me know ASAP if there is any concern here. Thank you.

Thanks govind@, I replied off-thread with our concerns. I hope we can get this merged for M69.

We really want the fix to be merge into M69 because there are other team are depending on this feature. 
The bug is about a feature that was shipped on M67, not a regression.  It's not going to affect any part other than the feature, so the merge should be safe

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

Approving merge for cl listed at #14 to M69 branch 3497 based on comment #21 and #22. Please merge ASAP. Thank you.

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

commit 55515662f98ae8e13715156a045eb9df07076dc8
Author: Ella Ge <eirage@chromium.org>
Date: Thu Aug 02 23:09:10 2018

Locked mouse event check ShouldGenerateAppCommand

When pointer is locked, we still need to check ShouldGenerateAppCommand
and set event handled so that won't generate unnecessary navigate
appcommand when kExtendedMouseButtons is enabled. This causes unable
to preventDefault on mouse back/forward buttons to prevent navigation
while pointer is locked.

Bug:  852709 
Change-Id: I5f75a68e36037210c38310f8d83825c5efdf311e
Reviewed-on: https://chromium-review.googlesource.com/1148886
Commit-Queue: Ella Ge <eirage@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#577889}(cherry picked from commit c18ebe7a9d694b01b05ca77644518f99087fb379)
Reviewed-on: https://chromium-review.googlesource.com/1161227
Reviewed-by: Ella Ge <eirage@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#362}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/55515662f98ae8e13715156a045eb9df07076dc8/content/browser/renderer_host/render_widget_host_view_event_handler.cc
[add] https://crrev.com/55515662f98ae8e13715156a045eb9df07076dc8/third_party/WebKit/LayoutTests/external/wpt/pointerlock/mouse_buttons_back_forward-manual.html
[add] https://crrev.com/55515662f98ae8e13715156a045eb9df07076dc8/third_party/WebKit/LayoutTests/external/wpt_automation/pointerlock/mouse_buttons_back_forward-manual-automation.js