From 6678244978c6547bbbaf1e62e215451713f8ebf6 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Mon, 12 Feb 2024 01:42:34 +0000 Subject: [PATCH] Bug 1878273 - Make `Shift` + right click should work as a right click without `Shift` if contextmenu should forcibly be open r=emilio We have a pref, `dom.event.contextmenu.shift_suppresses_event`, which is default to make `Shift` + right click forcibly open the context menu without dispatching `contextmenu` event to the web. When user wants to use this feature, they may (or must) assume that it works as a right click (without `Shift`) except whether it overrides `contextmenu` event listener of the web app. Therefore, `Selection` should be collapsed at the click point instead of expanding to the click point. Unfortunately, we need to consider it at `mousedown`, not `mouseup` nor `contextmenu`. Therefore, `Shift` state may mismatch with the actual state at `mouseup` and `mousedown`/`poinerdown` listeners of web apps may assume it will expand selection, but I think that we cannot solve these issues. Differential Revision: https://phabricator.services.mozilla.com/D201054 --- .../test_selection_after_right_click.html | 112 ++++++++++++++++++ layout/generic/nsIFrame.cpp | 13 +- ...th-non-primary-mouse-button.tentative.html | 30 ++++- 3 files changed, 147 insertions(+), 8 deletions(-) diff --git a/dom/events/test/test_selection_after_right_click.html b/dom/events/test/test_selection_after_right_click.html index 7a41f11b99ed..3661db9c694e 100644 --- a/dom/events/test/test_selection_after_right_click.html +++ b/dom/events/test/test_selection_after_right_click.html @@ -380,6 +380,118 @@ SimpleTest.waitForFocus(async () => { })` ); + // If Shift + right click should forcibly open context menu, users may want the click to work as + // same as without Shift. + await SpecialPowers.pushPrefEnv({ + set: [ + ["ui.mouse.right_click.collapse_selection.stop_if_non_collapsed_selection", true], + ["ui.mouse.right_click.collapse_selection.stop_if_non_editable_node", false], + ["ui.mouse.right_click.move_caret.stop_if_in_focused_editable_node", false], + ["dom.event.contextmenu.shift_suppresses_event", true], + ], + }); + nonEditableDiv.innerHTML = "bold italic"; + getSelection().collapse(nonEditableDiv.querySelector("b").firstChild, 0); + synthesizeMouseAtCenter(nonEditableDiv.querySelector("i"), {shiftKey: true, button: 2}); + ok( + getSelection().isCollapsed, + `Selection should be collapsed by a Shift + right click on non-editable node when it does not open context menu (${ + getRangeDescription(getSelection().getRangeAt(0)) + })` + ); + is( + getSelection().focusNode, + nonEditableDiv.querySelector("i").firstChild, + `Selection should be collapsed at the click point by a Shift + right click on non-editable node when it does not open context menu (${ + getRangeDescription(getSelection().getRangeAt(0)) + })` + ); + + editableDiv.innerHTML = "bold italic"; + getSelection().collapse(editableDiv.querySelector("b").firstChild, 0); + synthesizeMouseAtCenter(editableDiv.querySelector("i"), {shiftKey: true, button: 2}); + ok( + getSelection().isCollapsed, + `Selection should be collapsed by a Shift + right click on editable node when it does not open context menu (${ + getRangeDescription(getSelection().getRangeAt(0)) + })` + ); + is( + getSelection().focusNode, + editableDiv.querySelector("i").firstChild, + `Selection should be collapsed at the click point by a Shift + right click on editable node when it does not open context menu (${ + getRangeDescription(getSelection().getRangeAt(0)) + })` + ); + + input.focus(); + input.setSelectionRange(0, 0); + synthesizeMouseAtCenter(input, {shiftKey: true, button: 2}); + ok( + input.selectionStart == input.selectionEnd, + `Selection in should be collapsed by a Shift + right click when it does not open context menu (got: ${ + input.selectionStart + } - ${input.selectionEnd})` + ); + isnot( + input.selectionStart, + 0, + `Selection in should be collapsed at the click point by a Shift + right click when it does not open context menu (got: ${ + input.selectionStart + } - ${input.selectionEnd})` + ); + input.blur(); + + // If Shift + right click should open context menu, users may want the click to work as + // same as a left click. + await SpecialPowers.pushPrefEnv({ + set: [ + ["ui.mouse.right_click.collapse_selection.stop_if_non_collapsed_selection", true], + ["ui.mouse.right_click.collapse_selection.stop_if_non_editable_node", false], + ["ui.mouse.right_click.move_caret.stop_if_in_focused_editable_node", false], + ["dom.event.contextmenu.shift_suppresses_event", false], + ], + }); + nonEditableDiv.innerHTML = "bold italic"; + getSelection().collapse(nonEditableDiv.querySelector("b").firstChild, 0); + synthesizeMouseAtCenter(nonEditableDiv.querySelector("i"), {shiftKey: true, button: 2}); + is( + getRangeDescription(getSelection().getRangeAt(0)), + getRangeDescription({ + startContainer: nonEditableDiv.querySelector("b").firstChild, + startOffset: 0, + endContainer: nonEditableDiv.querySelector("i").firstChild, + endOffset: getSelection().focusOffset, + }), + `Selection should be extended by a Shift + right click on non-editable node when it should open context menu` + ); + + editableDiv.innerHTML = "bold italic"; + getSelection().collapse(editableDiv.querySelector("b").firstChild, 0); + synthesizeMouseAtCenter(editableDiv.querySelector("i"), {shiftKey: true, button: 2}); + is( + getRangeDescription(getSelection().getRangeAt(0)), + getRangeDescription({ + startContainer: editableDiv.querySelector("b").firstChild, + startOffset: 0, + endContainer: editableDiv.querySelector("i").firstChild, + endOffset: getSelection().focusOffset, + }), + `Selection should be extended by a Shift + right click on editable node when it should open context menu` + ); + + input.focus(); + input.setSelectionRange(0, 0); + synthesizeMouseAtCenter(input, {shiftKey: true, button: 2}); + isnot( + input.selectionStart, + input.selectionEnd, + `Selection in should be extended by a Shift + right click when it should open context menu (got: ${ + input.selectionStart + } - ${input.selectionEnd})` + ); + input.blur(); + SimpleTest.finish(); }); diff --git a/layout/generic/nsIFrame.cpp b/layout/generic/nsIFrame.cpp index e538da6ed0eb..f4684e28a6f7 100644 --- a/layout/generic/nsIFrame.cpp +++ b/layout/generic/nsIFrame.cpp @@ -4735,7 +4735,9 @@ nsresult nsIFrame::MoveCaretToEventPoint(nsPresContext* aPresContext, return NS_ERROR_FAILURE; } - if (aMouseEvent->mButton == MouseButton::eSecondary && + const bool isSecondaryButton = + aMouseEvent->mButton == MouseButton::eSecondary; + if (isSecondaryButton && !MovingCaretToEventPointAllowedIfSecondaryButtonEvent( *frameselection, *aMouseEvent, *offsets.content, // When we collapse selection in nsFrameSelection::TakeFocus, @@ -4833,7 +4835,14 @@ nsresult nsIFrame::MoveCaretToEventPoint(nsPresContext* aPresContext, const nsFrameSelection::FocusMode focusMode = [&]() { // If "Shift" and "Ctrl" are both pressed, "Shift" is given precedence. This // mimics the old behaviour. - if (aMouseEvent->IsShift()) { + const bool isShift = + aMouseEvent->IsShift() && + // If Shift + secondary button press shoud open context menu without a + // contextmenu event, user wants to open context menu like as a + // secondary button press without Shift key. + !(isSecondaryButton && + StaticPrefs::dom_event_contextmenu_shift_suppresses_event()); + if (isShift) { // If clicked in a link when focused content is editable, we should // collapse selection in the link for compatibility with Blink. if (isEditor) { diff --git a/testing/web-platform/tests/selection/contenteditable/modifying-selection-with-non-primary-mouse-button.tentative.html b/testing/web-platform/tests/selection/contenteditable/modifying-selection-with-non-primary-mouse-button.tentative.html index cb2e44295a94..79fc52ac7dbf 100644 --- a/testing/web-platform/tests/selection/contenteditable/modifying-selection-with-non-primary-mouse-button.tentative.html +++ b/testing/web-platform/tests/selection/contenteditable/modifying-selection-with-non-primary-mouse-button.tentative.html @@ -128,6 +128,11 @@ promise_test(async () => { resetEditor(); editor.focus(); selection.collapse(span1.firstChild, 2); + let contextmenuFired = false; + function onContextMenu() { + contextmenuFired = true; + } + document.addEventListener("contextmenu", onContextMenu, {capture: true}); let actions = new test_driver.Actions(); await actions .pointerMove(0, 0) @@ -137,13 +142,26 @@ promise_test(async () => { .pointerUp({button: getButtonType(actions)}) .keyUp("\uE008") .send(); + document.removeEventListener("contextmenu", onContextMenu, {capture: true}); - assert_equals(selection.anchorNode, span1.firstChild, - "Selection#anchorNode should keep in the first element"); - assert_equals(selection.anchorOffset, 2, - "Selection#anchorNode should keep at 2 of the first element"); - assert_equals(selection.focusNode, span2.firstChild, - `Selection#focusNode should be in the second element which was clicked by ${button} button`); + if (button != "secondary" || contextmenuFired) { + assert_equals(selection.anchorNode, span1.firstChild, + "Selection#anchorNode should keep in the first element"); + assert_equals(selection.anchorOffset, 2, + "Selection#anchorNode should keep at 2 of the first element"); + assert_equals(selection.focusNode, span2.firstChild, + `Selection#focusNode should be in the second element which was clicked by ${button} button`); + } else { + // Special case for Firefox. Firefox users can forcibly open context menu + // with pressing shift key. In this case, users may want the secondary + // button click to work as without Shift key press. + assert_true(selection.isCollapsed, + `Selection should be collapsed after ${button} button click because contextmenu was not opened with Shift key`); + assert_equals(selection.focusNode, span2.firstChild, + `Selection should be collapsed in the second element which was clicked by ${ + button + } button because contextmenu was not opened with Shift key`); + } }, `Shift + ${button} click should extend the selection`); promise_test(async () => {