Closed Bug 1739079 Opened 3 years ago Closed 2 years ago

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

Categories

(Core :: DOM: Selection, defect)

Firefox 94
defect

Tracking

()

VERIFIED FIXED
96 Branch
Tracking Status
firefox-esr91 --- wontfix
firefox94 --- wontfix
firefox95 --- wontfix
firefox96 --- verified

People

(Reporter: virkki, Assigned: emilio)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

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

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.

Component: Untriaged → DOM: UI Events & Focus Handling
Product: Firefox → Core

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

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.

Severity: -- → S2
Status: UNCONFIRMED → NEW
Component: DOM: UI Events & Focus Handling → DOM: Selection
Ever confirmed: true

Do you have a link to the profile by chance?

Flags: needinfo?(continuation)
Flags: needinfo?(continuation)

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

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 :-)

Flags: needinfo?(emilio)
Regressed by: 1698705
Has Regression Range: --- → yes

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.

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...

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

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

Flags: needinfo?(krosylight)

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

When dragging inside the text node in that example,

  1. The selection is collapsed in textarea.
  2. nsFrameSelection::TakeFocus calls Selection::Extend with the text node inside the text area.
  3. nsContentUtils::ComparePoints thinks the text node is after the textarea node, and sets anchorOldFocusOrder as 0 and anchorNewFocusOrder as 1.
  4. 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?
  1. It calls SetAnchorFocusToRange with that range and that again calls SetAnchorFocusToRange, AddRangesForSelectableNodes, AddRangesForUserSelectableNodes, and IsUserSelectionCollapsed in order.
  2. 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.
  3. Since it's detected as non-editor, it calls ExcludeNonSelectableNodes.
  4. 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.
  5. Since it can never meet the textarea node, it should stop when it eventually walked over everything in the tree. It does not.
  6. 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.

Flags: needinfo?(krosylight)
Assignee: nobody → emilio
Flags: needinfo?(emilio)

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

mIsDone = mCurNode == nullptr;

For anon content.

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/da8af162cd1e
Make ContentIterator more resilient to unexpected situations. r=saschanaz
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/31740 for changes under testing/web-platform/tests
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 96 Branch
Upstream PR merged by moz-wptsync-bot

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.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: