1845241 - [contenteditable] `RightClick` move caret at the start of the `contenteditable` element

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P3)

Description

[pomili.lorenzo85]

Attachment: context_menu_right_click.html

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/115.0

Steps to reproduce:

open the attached file and right click on the child div

Actual results:

the caret is moved at the start of the contenteditable element

Expected results:

the caret should be moved where the click happens, like when you use left click

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::DOM: Editor' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Alice0775 White]

#1 regression:

Good build: A caret appears at the right mouse click position.
Bad build: The caret is not shown.
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=06b2977afb85&tochange=569a45bfb71c

Suspect of #1 regression: ab8c0ebe600b7687cba76d7c2a429772ac9c0830 Graeme McCutcheon — Bug 682338 - Focus context menu's target on platforms where the context menu is shown on mousedown, not where it's shown on mouseup. r=enndeakin

#2 regression

The build from ae1dfa192faf: The caret is not shown as same as the above bad build.
Bad build: The caret is shown at the contenteditable element.
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ae1dfa192faf&tochange=92c87e95915e

Suspect of #2 regression: 030d8d4684982327356a377cbb13c82b665ce992 Masayuki Nakano — Bug 1083629 Use nsHTMLEditor::IsAcceptableInputEvent() instead of nsEditor::IsDescendantOfEditorRoot() for checking if a mouse down event target is in focused HTML editor r=ehsan

Comment 3

[Peter Van der Beken [:peterv]]

Other browsers seem to select the word that the user clicks on.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Well, it seems that the patch was written with wrong assumption.

On the other hand, I'm not sure that it's right behavior that caret is not moved with a right click in caret browsing mode. There is no handler for it, that was what I didn't expect.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm, there was a request for making the old (and expected by the reporter here) behavior cancelable.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Oh, and the old behavior may be triggered in unexpected cases too.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

It seems that we can fix this with touching nsIFrame::HandleEvent. Taking a look...

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

The patch will change Selection result so I don't think we should/can fix this in ESR branches.

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit BugBot documentation.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

FYI: Safari works similar to Chrome, but select a word under the cursor.

Comment 11

[pomili.lorenzo85]

Hello,
just to be sure did you switch it to wontfix because you reschedule the fix?

I'm asking because with the current behvaior is not possible to understand where the user has clicked.
So in some cases is not possible to align the browsers behviors when you work on an WYSIWYG editor like TinyMCE

Comment 12

[Donal Meehan [:dmeehan]]

(In reply to pomili.lorenzo85 from comment #11)

Hello,
just to be sure did you switch it to wontfix because you reschedule the fix?
Hi, the bug is not resolved as wontfix. The current release and upcoming release is set to wontfix since we don't have a patch.
The bug is in the team's backlog.

Comment 13

[pomili.lorenzo85]

ok, thanks for the clarification :)

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1845241 - Make `nsIFrame::HandleEvent` move caret when secondary mouse button down r=Edgar!,emilio!,#dom-core

The other browsers move focus and Selection whe right click even if the
clicked element is not editable and even if there is a non-collapsed selection.
Fortunately, we already have similar code for the middle button press.
Therefore, we can make it run when the pressed button is the secondary button.

This also fixes bug 416546 and does not resurrect bug 709476.

However, this patch adds 2 prefs for making users customizable. Our traditional
behavior is, we never collapse non-collapses selection with a right click even
if clicked outside the selection. This allows users to open context menu for
selected text much easier. Therefore, even though the behavior is different
from the others, we should keep the traditional behavior, but some users may
want the other browsers' behavior instead. For them, this should be switchable
by a pref.

Additionally, I'm still not sure collapsing selection with a right click in
non-editable content especially for users using the caret browsing mode.
Therefore, for making things safer, this adds a pref to disable the new behavior
in the non-editable content.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/81712ea02609
Make `nsIFrame::HandleEvent` move caret when secondary mouse button down r=edgar,emilio,dom-core

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/42439 for changes under testing/web-platform/tests

Comment 17

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/81712ea02609

Comment 18

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 19

[Ardelean Oana, Desktop QA [:oardelean]]

Reproducible on a 2023-10-08 Nightly build on macOS 12.
Verified as fixed on Firefox 120.0b5 and Nightly 121.0a1 on macOS 12, Windows 10, Ubuntu 22.

Comment 20

[Dianna Smith [:diannaS] - on Leave, back on August 9th]

backed out for 120.0rc2 for causing bug 1864334
Backout link
Backout push