2014481 - Selecting text doesn't work when toggling popover or switching display

Status: VERIFIED FIXED

(Core :: DOM: Selection, defect)

Description

[Moritz Beier [:mbeier]]

Attachment: example.html

See the attached html as an example. When selecting text while showPopover is called or display is changed, nothing is selected. The example works fine in Chrome and Safari.

Comment 1

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

Set release status flags based on info from the regressing bug 1930931

:sefeng211, since you are the author of the regressor, bug 1930931, could you take a look?

For more information, please visit BugBot documentation.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Bug 2007147 was reviewed by Masayuki and Sean is not working at MoCo anymore, so ni?ing him as well. The good thing is that bug 1930931 wasn't terribly big so maybe it's not too bad to fix :)

Comment 3

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

Although I've not checked it with the debugger. Looks like that changing display of the parent of <input> causes reframing the <input> and the anonymous nodes are recreated. I guess that the problem of the testcase is caused by the recreation occurs during the drag which changes the selection range and the text control state fails to cache the expected selection range.

I wonder if this could be fixed by reuse the anonymous subtrees after the reframe. I think TextEditor can own that instead of the frame. However, I don't know whether it's okay for the native anonymous node management. Emilio, do you know about that?

(I guess we cannot fix this with a small patch.)

Comment 4

[Emilio Cobos Álvarez [:emilio]]

I think we should try to move the editor shadow tree into the DOM. Which is not the same as NAC but should be similar-ish. I recently did that for <select> in https://phabricator.services.mozilla.com/D280041. Would that work? We need that for field-sizing: content I think anyways.

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Let me try to hack up a basic version of it.

Comment 6

[Emilio Cobos Álvarez [:emilio]]

Attachment: WIP: Bug 2014481 - Fix mismatch between GetContentForEvent and EventTargetData::ComputeElementFromFrame.

So I dug into this, and bug 2016280 is worth it but unrelated.

The key issue here is that the editor has a scrollframe with <div>text,
and BeginTrackingDragGesture ends up using the text node rather than the
element.

But PresShell::HandleEvent ends up using the element via
ComputeElementFromFrame, so nsIFrame::HandleEvent keeps bailing out
because we're tracking the text node in the gesture.

This fixes it but not sure it's the right fix.

Comment 7

[Emilio Cobos Álvarez [:emilio]]

See above. I don't think it has anything to do with selection ranges. We are just checking against the wrong tracked content.

Masayuki can you check the commit message above and see if it's the right fix, or propose one?

Comment 8

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

Thank you for debugging! I believe that the direction of the patch must be right. However, it's a low level API so that some callers may be designed for the current behavior. So, before landing it, we need to check each user (not only the direct callers, sigh).

Comment 9

[Emilio Cobos Álvarez [:emilio]]

Could you take a look at that? You know the constraints better than me for sure...

Comment 10

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

Sure, but not right now, later...

Comment 11

[Andreas Farre [:farre]]

Emilio, are you planning to work on this? Would you mind assigning it and set a severity?

Comment 12

[Emilio Cobos Álvarez [:emilio]]

I was hoping Masayuki could take a look as described in comment 8+...

FWIW, here's an ongoing try run: https://treeherder.mozilla.org/jobs?repo=try&revision=6fa05c6ff615625b9b231ca4a1204f42061caaa2

Severity is probably S3, but it'd be good to remove the hack bug 2007147 introduced.

Comment 13

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 2014481 - Fix selection dragging when reframed. r=masayuki!

When reframed, our frame selection might be reinitialized, losing the
drag state. Fix that up. A bit hacky, better ideas welcome, but I think
it's ok.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/6c9fd4f99258
https://hg.mozilla.org/integration/autoland/rev/70fc9edca172
Fix selection dragging when reframed. r=masayuki

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/1d00c5108d70
https://hg.mozilla.org/integration/autoland/rev/c6df4cf811cf
Revert "Bug 2014481 - Fix selection dragging when reframed. r=masayuki" for causing build bustages at TextControlState

Comment 16

[Silaghi Andreea]

Backed out for causing build bustages at TextControlState
Backout Link
Push with failures
Failure Log
Failure line /builds/worker/checkouts/gecko/dom/html/TextControlState.cpp:X:7: error: arguments must all be strong refs or caller's parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument). 'mFrameSelection->' is neither.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/3f1228e02a3f
https://hg.mozilla.org/integration/autoland/rev/f86e640cbeb5
Fix selection dragging when reframed. r=masayuki

Comment 18

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/f86e640cbeb5

Comment 19

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

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox150 to wontfix.

For more information, please visit BugBot documentation.

Comment 20

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined/Reason for urgency: Fixes a compat regression in duckduckgo and has been hit by the frontend as well.
• Code covered by automated testing?: yes
• Fix verified in Nightly?: yes
• Needs manual QE testing?: yes
• Steps to reproduce for manual QE testing: comment 0 or dependent compat issue.
• Risk associated with taking this patch: low
• Explanation of risk level: Relatively minimal fix.
• String changes made/needed?: none
• Is Android affected?: yes

Comment 21

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 2014481 - Fix selection dragging when reframed.

When reframed, our frame selection might be reinitialized, losing the
drag state. Fix that up. A bit hacky, better ideas welcome, but I think
it's ok.

Original Revision: https://phabricator.services.mozilla.com/D292907

Comment 22

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/821137c12934
https://hg.mozilla.org/releases/mozilla-beta/rev/0b0134b9d1f6

Comment 23

[Mira Lobontiu (Android QA)]

Verified on the Firefox for Android Nightly 151.0a1, and on Beta 150.0b9, with a Pixel 6 (Android 17), and an Oppo Find n2 Flip (Android 15).

Comment 24

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

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

Comment 25

[Alin Ilea, Desktop Test Engineering]

Reproduced the issue in Release 149.0.2
Verified - Fixed in Beta 150.0b9(build id: 20260413090345) and Nightly 151.0a1(build id: 20260413220854). The text can be selected using the mouse dragging and also using the keyboard(shift+arrows) in the mentioned Fx versions. I'll leave the bug status unchanged for now, since ESR140 is still affected.

Comment 26

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

Upstream PR merged by moz-wptsync-bot