1737540 - can no longer type over some selections in unknown contenteditable element

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect, P3)

Description

[stef.busking]

Attachment: custom-contenteditable.html

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

Steps to reproduce:

Minimal repro at https://codepen.io/Bwrrp/pen/ExvYLwZ and in attachment.

• Place cursor at the end of the text, after the .
• Press shift-home to select the entire line
• Type a letter

Actual results:

In Firefox 92 and earlier, the selection is deleted and replaced with the typed letter. Starting from Firefox 93, nothing happens.

When using the Japanese IME built into Windows 10 set to Hiragana, composition does start, but the selected text is still not removed and the initial composition text is not visible. The IME window is sometimes positioned in an unexpected place, overlapping the text. Continuing to type into the IME in this way finally ends up prepending text rather than overtyping the selection. Again, this did work correctly in Firefox 92 and earlier.

Expected results:

Expected typing to immediately replace the selected text, whether composition is used or not.

Replacing the custom-element tag with a div makes this work as expected. If the contenteditable element only contains text, this also works. The combination of the unknown element and the nested inline seems to cause this to break.

No js is involved. The CSS in the repro case only serves to make placing the cursor easier, the unexpected behavior also occurs if this is removed and custom-element defaults to being inline.

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 revert this change in case you think the bot is wrong.

Comment 2

[Andreas Farre [:farre]]

Mozregression says that this is a regression from Bug 1726064

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=3db432e2820882f8a29b52e4a3a0baee9e8d19eb&tochange=7af78405aade4dbd64f4c713dc3feeed5d8ffa5b

Comment 3

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

Hmm, editor may not be able to refer the latest layout information, it means that editor can/should not refer CSS display value. Therefore, the unknown element is treated as inline and it's handled as error. I think that the caller's check (in PrepareToDeleteNonCollapsedRanges()) should be changed.

Comment 4

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

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

Comment 5

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

Attachment: Bug 1737540 - Make `AutoBlockElementsJoiner::PrepareToDeleteNonCollapsedRanges()` consider to delete given range when there is no editable block parent r=m_kato!

Currently, it assumes that first range is always in editable block parent,
but if the range is in an inline editing host, it's not true.

The given ranges should always be in same editing host at this time because of:

• https://searchfox.org/mozilla-central/rev/011ed92913b38e950977ab3fc56ae68a8f3bca12/editor/libeditor/HTMLEditorDeleteHandler.cpp#1031,1064
• https://searchfox.org/mozilla-central/rev/011ed92913b38e950977ab3fc56ae68a8f3bca12/editor/libeditor/HTMLEditorDeleteHandler.cpp#1080,1117

Therefore, it's enough to check whether the range crosses or not editing host
boundaries only with MOZ_ASSERT(_IF).

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d2060dd5c42a
Make `AutoBlockElementsJoiner::PrepareToDeleteNonCollapsedRanges()` consider to delete given range when there is no editable block parent r=m_kato

Comment 7

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

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

Comment 8

[Cristina Cozmuta (:CrissCozmuta)]

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

Comment 9

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

Upstream PR merged by moz-wptsync-bot

Comment 10

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

The patch landed in nightly and beta is affected.
:masayuki, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 11

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

This was not reported immediately. So I think that there is no web apps which hit this bug. And I'm afraid to uplift the patch to beta because the changed path is used in other cases too.

Comment 12

[stef.busking]

This is actually affecting the Fonto Editor (see for instance https://trial.fontoxml.com/), which makes heavy use of custom elements both inside and outside of contenteditable.

Comment 13

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

(In reply to stef.busking from comment #12)

This is actually affecting the Fonto Editor (see for instance https://trial.fontoxml.com/), which makes heavy use of custom elements both inside and outside of contenteditable.

Indeed, this bug causes sometimes cannot edit the text when I move caret with outline of the right side-bar.

Comment 14

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

Comment on attachment 9248837 [details]
Bug 1737540 - Make AutoBlockElementsJoiner::PrepareToDeleteNonCollapsedRanges() consider to delete given range when there is no editable block parent r=m_kato!

Beta/Release Uplift Approval Request

• User impact if declined: Fonto Editor (see for instance https://trial.fontoxml.com/) is not usable in some cases. I confirmed that when I move caret from outline of the right side bar, and type something, then select some characters, I cannot type text to replace the non-collapsed selection.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): The changed path runs when deletion in the other patterns because it considers which path should be used to delete under each situation. But the change is simple and I believe that it's reasonable.
• String changes made/needed: None

Comment 15

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

It seems that this fixes bug 1740492 too. If so, this is more important than I've expected.

Comment 16

[Dianna Smith [:diannaS]]

Comment on attachment 9248837 [details]
Bug 1737540 - Make AutoBlockElementsJoiner::PrepareToDeleteNonCollapsedRanges() consider to delete given range when there is no editable block parent r=m_kato!

Approved for 95.0b6

Comment 17

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ec90864503dd

Comment 18

[Catalin Sasca, Desktop QA [:csasca]]

Was able to reproduce the issue on macOS 10.15 under 95.0a1 (2021-10-25) by following the info provided in Comment 0.

The issue is fixed on Firefox 95.0b6 and Firefox 96.0a1 (2021-11-11). Tests were performed on macOS 10.15, Windows 10 and Ubuntu 20.04.