1739079 - Disabled textarea with pointer-events: none freezes browser on drag

Status: VERIFIED FIXED

(Core :: DOM: Selection, defect)

Description

[virkki]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/95.0.4638.54 Safari/537.36

Steps to reproduce:

1. Open https://jsbin.com/rufelafeve/edit
2. Try dragging the disabled text area

Actual results:

The browser freezes

Expected results:

Nothing should happen

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Core::DOM: UI Events & Focus Handling' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[virkki]

The issue was originally reported in https://github.com/vaadin/flow-components/issues/2296

Comment 3

[Andrew McCreight [:mccr8]]

This looks pretty bad. The content process for the page hits 100% CPU and didn't get killed even after I closed the tab.

I took a profile and it look like the time was being spent in selection code.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Do you have a link to the profile by chance?

Comment 5

[Andrew McCreight [:mccr8]]

https://share.firefox.dev/3k6Nteb

Comment 6

[Kagami Rosylight [:saschanaz] (they/them)]

It seeems like an infinite loop in nsRange::ExcludeNonSelectableNodes?

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Mozregression says: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=9d48260391c17cab1218c76164db5fa73f8706b7&tochange=4551319b3f4328d7175ef5111ea0c0debd0264b1

Given the change, likely a pre-existing bug that was uncovered by that, but... Anyways will look of course. Kagami if you have time to poke that'd also be awesome of course :-)

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Test-case from reporter.

Comment 9

[Kagami Rosylight [:saschanaz] (they/them)]

Taking a look. PreContentIterator loops infinitely as preOrderIter.Next() does nothing when no mCurNode exists. if (mIsDone || NS_WARN_IF(!mCurNode)) { was not enough, probably should be MOZ_ASSERT instead.

Comment 10

[Kagami Rosylight [:saschanaz] (they/them)]

So that's one problem but the core issue is that the range for the iterator starts from the text node and ends with the text area node. Naturally it can't just iterate into the shadow tree by checking siblings and thus the assumption is broken and the "last node" is never met, ᅟso the loop.

Not sure how to fix the core problem (do we really want to iterate into shadow tree?), but the hotfix would be to check if the "current node" actually exists and return early if not. (Edit: fix the iterator to check the resulting node exist and if not set the mIsDone flag)

Edit2: But then why does it only happen with disabled? More to dig later...

Comment 11

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

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

Comment 12

[virkki]

In case someone encounters this: one workaround is to set user-select: none to the slotted <textarea>: https://github.com/vaadin/web-components/pull/2981

Comment 13

[Kagami Rosylight [:saschanaz] (they/them)]

Sorry, what I reported in comment #10 was not 100% correct.

When dragging inside the text node in that example,

0. The selection is collapsed in textarea.
1. nsFrameSelection::TakeFocus calls Selection::Extend with the text node inside the text area.
2. nsContentUtils::ComparePoints thinks the text node is after the textarea node, and sets anchorOldFocusOrder as 0 and anchorNewFocusOrder as 1.
3. It enters *anchorOldFocusOrder == 0 && *anchorNewFocusOrder > 0 branch, where the range start becomes the text node and the range end becomes the textarea, and calls SetDirection(eDirPrevious).

• This is weird, why not just set the range end as the text node and and set the direction as forward?

4. It calls SetAnchorFocusToRange with that range and that again calls SetAnchorFocusToRange, AddRangesForSelectableNodes, AddRangesForUserSelectableNodes, and IsUserSelectionCollapsed in order.
5. The last one calls UserSelectRangesToAdd where the behavior changes when the range's start and end containers are all editor nodes. Here, having disabled and pointer-events: none somehow sets the text node as non-editor-node, and the behavior change starts.
6. Since it's detected as non-editor, it calls ExcludeNonSelectableNodes.
7. Here, PreContentIterator tries to iterate within the range. As step 3 set the range backward, it iterates from the text node to the textarea node, without knowing the direction is actually backward. That obviously won't work.
8. Since it can never meet the textarea node, it should stop when it eventually walked over everything in the tree. It does not.
9. Boom.

But all of this require the text area be in that specific tree in the example, or step 1 instead gets HTMLBodyElement or ShadowRoot, which is seemingly decided by nsIFrame::MoveCaretToEventPoint and Emilio should know better than me about this.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1739079 - Make ContentIterator more resilient to unexpected situations. r=saschanaz,mbrodesser,masayuki

This shouldn't change behavior in non-hanging cases and should fix the
hang. We had similar hackish:

mIsDone = mCurNode == nullptr;

For anon content.

Comment 15

[Emilio Cobos Álvarez (:emilio)]

https://treeherder.mozilla.org/jobs?repo=try&revision=431c41678f05115c652da7eb07636da3806710a1

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/da8af162cd1e
Make ContentIterator more resilient to unexpected situations. r=saschanaz

Comment 17

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

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

Comment 18

[Iulian Moraru]

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

Comment 19

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

Upstream PR merged by moz-wptsync-bot

Comment 20

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Reproduced the issue with Firefox 96.0a1 (20211104214758) on Windows 10x64. Tab freezes when dragging the disabled text area from the attaches test case.
Verified fixed with 96.0a1 (20211202094249) on Windows 10x64, macOS 10.15 and Ubuntu 20.04. Dragging the disabled text area from the attached test case does not freeze the browser.