Closed Bug 1524266 Opened 2 years ago Closed 2 years ago

select tags in contentEditable can't be selected for removal


(Core :: Layout, defect)

65 Branch
Not set



Tracking Status
firefox67 --- fixed


(Reporter: joerg, Assigned: emilio)




(3 files)

Attached file select.html

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

Steps to reproduce:

Open the attached test case. Select the <select> individually or with the surrounding letters (but without using select-all). Try to delete the selection.

Now use select-all and try to delete again.

Actual results:

In both cases the <select> should be removed from the editor block.

Expected results:

If the mouse selection contains the <select> and wasn't done using C-A, it can't be removed. With C-A, it can be removed.

This is a regression from Firefox 64. is the source of the regression according to mozregression.

Thanks for the report, will take a look :)

Blocks: 1506547
Component: Untriaged → Layout
Ever confirmed: true
Flags: needinfo?(emilio)
Product: Firefox → Core

So, is a patch that I think improves the behavior of contenteditable="true" -> contenteditable="false" considerably.

Need to check what our test suite thinks of that, write some tests, and send it for review if all the above goes well.

A possible workaround you could apply to that test-case for now could be something like:

span[contenteditable="false"], span[contenteditable="false"] select {
  -moz-user-select: all;

For example. The mere reason this ever worked, looking at our code, seems to be luck.

That looks pretty close! Let me write some tests.

Assignee: nobody → emilio

This makes our behavior a bit closer to Blink / WebKit.

This patch fixes multiple issues:

First, fixes the caret movement getting stuck on a <select> element inside an
editor. This is because of the IsRootOfAnonymousSubtree() check that I'm
removing. Instead of that, consider NAC unselectable in UsedUserSelect, just
like generated content. This makes us jump across it correctly, and doesn't
regress the test-case that was added in bug 989012.

Second, it allows to select nodes with user-select: none as long as you're on an
editor. This matches WebKit and Blink. It's something you could do earlier
regardless with user-select: all on the parent, which is why the reporter's
test-case worked before my patch. I think being able to jump across these and
delete them on an editor is the right thing to do.

It adds tests for all this plus the same thing working for non-editable contents
(there was no pre-existing test for that).

Flags: needinfo?(emilio)
Attached file Testcase #2

Sigh, it looks like Phabricator discarded my review comment...

Pushed by
Should be able to delete non-selectable and non-editable content in a contenteditable subtree. r=mats
Pushed by
Should be able to delete non-selectable and non-editable content in a contenteditable subtree. r=mats

Thanks, I removed the UsedUserSelect change, since it causes trouble for the anonymous <input> in <input type="number">.

Flags: needinfo?(emilio)
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.