Skip to content

Commit

Permalink
Bug 1878273 - Make Shift + right click should work as a right click…
Browse files Browse the repository at this point in the history 
… 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
  • Loading branch information
@masayuki-nakano
masayuki-nakano committed Feb 12, 2024
1 parent 5a2f926 commit 6678244
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 8 deletions.
112 changes: 112 additions & 0 deletions dom/events/test/test_selection_after_right_click.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,118 @@
})`
);

// 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 = "<b>bold</b> <i>italic</i>";
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 = "<b>bold</b> <i>italic</i>";
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 <input> 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 <input> 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 = "<b>bold</b> <i>italic</i>";
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 = "<b>bold</b> <i>italic</i>";
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 <input> should be extended by a Shift + right click when it should open context menu (got: ${
input.selectionStart
} - ${input.selectionEnd})`
);
input.blur();

SimpleTest.finish();
});
</script>
Expand Down
13 changes: 11 additions & 2 deletions layout/generic/nsIFrame.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down
30 changes: 24 additions & 6 deletions ...election/contenteditable/modifying-selection-with-non-primary-mouse-button.tentative.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,11 @@
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)
Expand All @@ -137,13 +142,26 @@
.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 <span> element");
assert_equals(selection.anchorOffset, 2,
"Selection#anchorNode should keep at 2 of the first <span> element");
assert_equals(selection.focusNode, span2.firstChild,
`Selection#focusNode should be in the second <span> element which was clicked by ${button} button`);
if (button != "secondary" || contextmenuFired) {
assert_equals(selection.anchorNode, span1.firstChild,
"Selection#anchorNode should keep in the first <span> element");
assert_equals(selection.anchorOffset, 2,
"Selection#anchorNode should keep at 2 of the first <span> element");
assert_equals(selection.focusNode, span2.firstChild,
`Selection#focusNode should be in the second <span> 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 <span> 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 () => {
Expand Down

0 comments on commit 6678244

Please sign in to comment.