1633399 - [twitter] composing after selecting text that spans a newline inserts duplicate text (most noticeable with Japanese/Korean IMEs)

Status: NEW

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

Description

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

1. Access twitter.com
2. Type "가" and press enter key.
3. Tap Control+Shift+Home to select all texts including newline.
4. Type "가" again and press space key.

Expected: "가"
Actual: Twitter crashes - "Something went wrong, but don’t fret — it’s not your fault."

This also causes a bug on Chrome but it doesn't crash. I guess we need to contact Twitter.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Twitter crashes or some child process of Firefox crashes?
(I don't know what "Twitter crashes" means :) )

Maybe this is a tech evangelism bug?

Comment 2

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

It's Twitter that crashes, not the Firefox tab itself. Seemingly an exception occurs from a mutation observer callback.

exports (https://abs.twimg.com/responsive-web/web/bundle.RichTextCompose.3bbd68a4.js#9)
registerMutation (https://abs.twimg.com/responsive-web/web/bundle.RichTextCompose.3bbd68a4.js#9)
registerMutations (https://abs.twimg.com/responsive-web/web/bundle.RichTextCompose.3bbd68a4.js#9)
e (https://abs.twimg.com/responsive-web/web/bundle.RichTextCompose.3bbd68a4.js#9)
e (https://abs.twimg.com/responsive-web/web/bundle.RichTextCompose.3bbd68a4.js#9)
onCompositionStart (https://abs.twimg.com/responsive-web/web/bundle.RichTextCompose.3bbd68a4.js#9)
onCompositionStart (https://abs.twimg.com/responsive-web/web/bundle.RichTextCompose.3bbd68a4.js#9)
_buildHandler (https://abs.twimg.com/responsive-web/web/bundle.RichTextCompose.3bbd68a4.js#1)
ta (https://abs.twimg.com/responsive-web/web/vendors~main.e0482f54.js#65)
unstable_runWithPriority (https://abs.twimg.com/responsive-web/web/main.82ef4e44.js#10)
ta (https://abs.twimg.com/responsive-web/web/vendors~main.e0482f54.js#65)
unstable_flushControlled (https://abs.twimg.com/responsive-web/web/vendors~main.e0482f54.js#65)
_buildHandler (https://abs.twimg.com/responsive-web/web/bundle.RichTextCompose.3bbd68a4.js#1)
exports (https://abs.twimg.com/responsive-web/web/bundle.RichTextCompose.3bbd68a4.js#9)
_buildHandler (https://abs.twimg.com/responsive-web/web/bundle.RichTextCompose.3bbd68a4.js#1)
ta (https://abs.twimg.com/responsive-web/web/vendors~main.e0482f54.js#65)
unstable_runWithPriority (https://abs.twimg.com/responsive-web/web/main.82ef4e44.js#10)
ta (https://abs.twimg.com/responsive-web/web/vendors~main.e0482f54.js#65)
unstable_flushControlled (https://abs.twimg.com/responsive-web/web/vendors~main.e0482f54.js#65)
_buildHandler (https://abs.twimg.com/responsive-web/web/bundle.RichTextCompose.3bbd68a4.js#1)
a (https://abs.twimg.com/responsive-web/web/vendors~main.e0482f54.js#65)
p (https://abs.twimg.com/responsive-web/web/vendors~main.e0482f54.js#65)
O (https://abs.twimg.com/responsive-web/web/vendors~main.e0482f54.js#65)
O (https://abs.twimg.com/responsive-web/web/vendors~main.e0482f54.js#65)
C (https://abs.twimg.com/responsive-web/web/vendors~main.e0482f54.js#65)
P (https://abs.twimg.com/responsive-web/web/vendors~main.e0482f54.js#65)
R (https://abs.twimg.com/responsive-web/web/vendors~main.e0482f54.js#65)
kn (https://abs.twimg.com/responsive-web/web/vendors~main.e0482f54.js#65)
Ia (https://abs.twimg.com/responsive-web/web/vendors~main.e0482f54.js#65)
ze (https://abs.twimg.com/responsive-web/web/vendors~main.e0482f54.js#65)
Cn (https://abs.twimg.com/responsive-web/web/vendors~main.e0482f54.js#65)
En (https://abs.twimg.com/responsive-web/web/vendors~main.e0482f54.js#65)
pr (https://abs.twimg.com/responsive-web/web/vendors~main.e0482f54.js#65)

Comment 3

[Hsin-Yi Tsai (she/her)[:hsinyi]]

This may worth webcompat team's attention.

Comment 4

[Mike Taylor [:miketaylr]]

Non-ascii text + selection reminds me of Bug 1625475... I'll email Twitter today.

Comment 5

[Mike Taylor [:miketaylr]]

Attachment: screenshot of crashed twitter tab

Here's a screenshot of what this looks like. Note: I had to install the Korean IME to get this to reproduce, it's not enough to merely copy and paste the chars.

Comment 6

[Mike Taylor [:miketaylr]]

I sent an email to our mozilla-twitter mailing list.

Comment 7

[charlie.croom]

Can you test on https://draftjs.org/ ? Look for any errors in the console. We generally just use that as our editor without too many frills on top, so any issues are likely with that editor itself. We're a minor version behind, so it's possible upgrading might help.

Comment 8

[Mike Taylor [:miketaylr]]

Attachment: screenshot of draftjs demo

OK, yeah. I think Twitter is off the hook, this manages to nuke the entire DraftJS page. :)

Comment 9

[Mike Taylor [:miketaylr]]

I filed https://github.com/facebook/draft-js/issues/2412.

Comment 10

[Hsin-Yi Tsai (she/her)[:hsinyi]]

Masayuki, is this another variant of bug 1739489?

Comment 11

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

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #10)

Masayuki, is this another variant of bug 1739489?

Could be, but not directly same thing since Windows does not use the new path for inputting commit string without composition events.

Comment 12

[Hsin-Yi Tsai (she/her)[:hsinyi]]

Kagami, do you still see this issue?

Comment 13

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Yes, but it does not crash anymore but instead cleans all the text away unexpectedly.

Comment 14

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

(Still reproducible. Just wanted to say as there's a new see-also)

Comment 16

[Jeff Muizelaar [:jrmuizel]]

Kagami, can you still reproduce this?

Comment 17

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Yes on Nightly 2024-10-15.

Comment 18

[Randell Jesup [:jesup] (needinfo me)]

DraftJS was archived 2 years ago or more. It will not get any fixes other than maybe security. Is X still using DraftJS? If they are, it's unlikely this gets fixed until they stop using it. If they were still using it 6 months ago, they are probably ignoring that it's deprecated/mothballed, and with Musk's staffing levels, it's unlikely to be addressed.

Also, this will only affect Korean-language users of X (apparently), so that minimizes the impact even more.

Likely this should be marked blocked, and/or downgraded. We could reach out to twitter/x but I wouldn't bet on this helping, since they'd have to rewrite to remove draftjs very likely.

Marking blocked for now, dennis can make any further adjustments

Comment 19

[Jeff Muizelaar [:jrmuizel]]

Moving back to needs-diagnosis, as I don't see a diagnosis here. Even if DraftJS has a bug the experience is much worse in Firefox.

Comment 20

[Jeff Muizelaar [:jrmuizel]]

Masayuki, can you figure out why draftjs is crashing in Firefox but not in Chrome/WebKit?

Comment 21

[Hsin-Yi Tsai (she/her)[:hsinyi]]

The issue I am currently seeing is as what Kagami mentioned in comment 13; that is, it does not crash anymore but instead cleans all the text away unexpectedly.

Comment 22

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

I'll try to investigate this more from C++ side, but anyway, this happens only when users types text at start of a link without putting a space before a link. Without doing that, the script anyway runs to update the link target, so, I guess most users do not meet this symptom with putting a space.

Comment 23

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

Oops, I was confused with bug 1630735...

It's hard to debug this bug because I also see odd behavior on Chrome. When I type "あ" with selecting the previous composition and the line break on Chrome, the line break is also appended unexpectedly...

As far as I've checked, Selection range is same, but we don't delete selected text without stripping the container element (<div data-content="true"> in this case). That might affect, but it seems that Chrome also works as so (otherwise, replacing the only text in a paragraph with composing string will delete the paragraph itself).

Comment 24

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

The log at replacing select text with new composition string and commit it.

I/SelectionAPI 21a5f542bd0 Selection::mozilla::dom::Selection::AddRangeJS(aRange={ mStart=mEnd={ mParent=0000021A64BEE940 (span.div[class="public-DraftStyleDefault-block public-DraftStyleDefault-ltr"].div.div.div[class="notranslate public-DraftEditor-content"][contenteditable="true"].<snip>.body.html.#document, Length()=1), mRef=null, mOffset=0, mIsMutationObserved=true }, mIsGenerated=false, mCalledByJS=true, mIsDynamicRange=true })
I/SelectionAPI 21a5f542bd0 Selection::mozilla::dom::Selection::ExtendJS(aContainer=#text.span.span.div[class="public-DraftStyleDefault-block public-DraftStyleDefault-ltr"].div.div.div[class="notranslate public-DraftEditor-content"][contenteditable="true"].<snip>.body.html.#document, aOffset=0)
I/EditorTextInput 21a6c59cc00 HTMLEditor::OnCompositionStart(aCompositionStartEvent={ mData="あ\n"}), mComposition=0
I/EditorTextInput 21a6c59cc00 HTMLEditor::OnCompositionChange(aCompositionChangeEvent={ mData="あ", IsFollowedByCompositionEnd()=false }), mComposition=21a649f4840
I/EditorEvent 21a6c59cc00 HTMLEditor: Dispatching "beforeinput" event: { inputType="EditorInputType::eInsertCompositionText" }...
I/EditorEvent 21a6c59cc00 HTMLEditor: Dispatched "beforeinput" event: { inputType="EditorInputType::eInsertCompositionText" }, defaultPrevented=false
I/EditorTextInput 21a6c59cc00 HTMLEditor::HandleInsertText(aInsertionString="あ", aPurpose=InsertTextFor::CompositionStart)
I/SelectionAPI 21a5f542bd0 nsRange::nsRange::DoSetRange(aStartBoundary={ mParent=0000021A64BEE280 (div.div[class="notranslate public-DraftEditor-content"][contenteditable="true"].<snip>.body.html.#document, Length()=2), mRef=0000000000000000, mOffset=0, mIsMutationObserved=true }, aEndBoundary={ mParent=0000021A64BEE940 (span.div[class="public-DraftStyleDefault-block public-DraftStyleDefault-ltr"].div.div.div[class="notranslate public-DraftEditor-content"][contenteditable="true"].<snip>.body.html.#document, Length()=1), mRef=0000000000000000, mOffset=0, mIsMutationObserved=true }, aNotInsertedYet=false)
I/SelectionAPI 21a5f542bd0 nsRange::nsRange::DoSetRange(aStartBoundary=aEndBoundary={ mParent=0000021A64BEE280 (div.div[class="notranslate public-DraftEditor-content"][contenteditable="true"].<snip>.body.html.#document, Length()=1), mRef=0000000000000000, mOffset=0, mIsMutationObserved=true }, aNotInsertedYet=false)
I/SelectionAPI 21a5f542bd0 Selection::mozilla::dom::Selection::CollapseInLimiter(aPoint={ mParent=0000021A64BEE280 (div.div[class="notranslate public-DraftEditor-content"][contenteditable="true"].<snip>.body.html.#document, Length()=0), mOffset=0, mIsMutationObserved=false })
I/SelectionAPI 21a5f542bd0 Selection::mozilla::dom::Selection::CollapseInLimiter(aPoint={ mParent=0000021A64BEE280 (div.div[class="notranslate public-DraftEditor-content"][contenteditable="true"].<snip>.body.html.#document, Length()=0), mOffset=0, mIsMutationObserved=false })
I/EditorTextInput 21a6c59cc00 HTMLEditor::HandleInsertText(), pointToInsert={ mParent=0000021A64BEE280 (div.div[class="notranslate public-DraftEditor-content"][contenteditable="true"].<snip>.body.html.#document, Length()=0), mChild=0000000000000000, mOffset=0, mIsChildInitialized=true, mInterlinePosition=InterlinePosition::Undefined }
I/SelectionAPI 21a5f542bd0 Selection::mozilla::dom::Selection::CollapseInLimiter(aPoint={ mParent=0000021A64B33380 (#text.div.div[class="notranslate public-DraftEditor-content"][contenteditable="true"].<snip>.body.html.#document, Length()=1), mRef=0000000000000000, mOffset=1, mIsMutationObserved=true })
I/EditorEvent 21a6c59cc00 HTMLEditor: Dispatching "input" event: { inputType="EditorInputType::eInsertCompositionText" }...
D/EditorEvent 21a6c59cc00 HTMLEditor: Dispatched "input" event: { inputType="EditorInputType::eInsertCompositionText" }
JavaScript error: https://abs.twimg.com/responsive-web/client-web/vendor-3dfac8a4.346d055a.js, line 10: Error: Got unexpected null or undefined
JavaScript error: https://abs.twimg.com/responsive-web/client-web/vendor-3dfac8a4.346d055a.js, line 10: Error: Got unexpected null or undefined
I/EditorTextInput 21a6c59cc00 HTMLEditor::OnCompositionChange(aCompositionChangeEvent={ mData="あ", IsFollowedByCompositionEnd()=true }), mComposition=21a649f4840
I/EditorEvent 21a6c59cc00 HTMLEditor: Dispatching "beforeinput" event: { inputType="EditorInputType::eInsertCompositionText" }...
I/EditorEvent 21a6c59cc00 HTMLEditor: Dispatched "beforeinput" event: { inputType="EditorInputType::eInsertCompositionText" }, defaultPrevented=false
I/EditorTextInput 21a6c59cc00 HTMLEditor::HandleInsertText(aInsertionString="あ", aPurpose=InsertTextFor::CompositionEnd)
I/EditorTextInput 21a6c59cc00 HTMLEditor::HandleInsertText(), pointToInsert={ mParent=0000021A64B33380 (#text.div.div[class="notranslate public-DraftEditor-content"][contenteditable="true"].<snip>.body.html.#document, (begins with=""), Length()=1), mChild=0000000000000000, mOffset=0, mIsChildInitialized=true, mInterlinePosition=InterlinePosition::Undefined }
I/SelectionAPI 21a5f542bd0 nsRange::nsRange::DoSetRange(aStartBoundary=aEndBoundary={ mParent=0000021A64B33380 (#text.div.div[class="notranslate public-DraftEditor-content"][contenteditable="true"].<snip>.body.html.#document, Length()=1), mRef=0000000000000000, mOffset=0, mIsMutationObserved=true }, aNotInsertedYet=false)
I/SelectionAPI 21a5f542bd0 Selection::mozilla::dom::Selection::CollapseInLimiter(aPoint={ mParent=0000021A64B33380 (#text.div.div[class="notranslate public-DraftEditor-content"][contenteditable="true"].<snip>.body.html.#document, Length()=1), mRef=0000000000000000, mOffset=1, mIsMutationObserved=true })
WARNING: '!newCompositionStartPoint.IsSet()', file M:/src/editor/libeditor/HTMLEditSubActionHandler.cpp:1302
I/EditorEvent 21a6c59cc00 HTMLEditor: Dispatching "input" event: { inputType="EditorInputType::eInsertCompositionText" }...
D/EditorEvent 21a6c59cc00 HTMLEditor: Dispatched "input" event: { inputType="EditorInputType::eInsertCompositionText" }
JavaScript error: https://abs.twimg.com/responsive-web/client-web/vendor-3dfac8a4.346d055a.js, line 10: Error: Got unexpected null or undefined
I/EditorTextInput 21a6c59cc00 HTMLEditor::OnCompositionEnd(aCompositionEndEvent={ mData="あ"}), mComposition=21a649f4840
I/EditorEvent 21a6c59cc00 HTMLEditor: Dispatching "input" event: { inputType="EditorInputType::eInsertCompositionText" }...
D/EditorEvent 21a6c59cc00 HTMLEditor: Dispatched "input" event: { inputType="EditorInputType::eInsertCompositionText" }
[Parent 63984, Main Thread] WARNING: 'mSelection->mHasRange', file M:/src/widget/ContentCache.cpp:831
[Parent 63984, Main Thread] WARNING: 'queryTextRectEvent.Failed()', file M:/src/widget/windows/TSFTextStore.cpp:2020

Comment 25

[Randell Jesup [:jesup] (needinfo me)]

Masayuki, Tom, is this something we could conceivably site-patch (modify their draftjs code to make it work)?

Comment 26

[Thomas Wisniewski [:twisniewski]]

Possibly, but I suspect that I will need Masayuki-san (or others) to find out the root cause, and how their JS or our internals need to be changed to fix it, as this seems a bit beyond me.

Comment 27

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

I have no more idea what's going on. (And I don't have much time for now because of the remaining Interop work of Pointer Events.)

Comment 28

[charlie.croom]

For what it's worth, it was years ago but I vaguely remember that comparing the ordering of the raw event stream coming in via the editor in Firefox vs other browsers...the ordering of certain events was slightly different. I can't remember exactly what, but things like compositionend being swapped with input or something to that effect. I know we talked with the FB draftjs folks at the time too, but I think handling the various orderings was hard on that end.

Comment 29

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Given it's reproducible on draftjs demo page, and the library is not changing anymore, perhaps we can make a forked library with rich logs and test on it, so that the difference with Chrome can be more visible.

Comment 30

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

From my newly built page: https://saschanaz.github.io/draft-js/ (for non-minified version please build locally. Would need some change to make the webpage non-minified)

STR is basically comment #0 but only partially:

1. Type "ㄱ" and press enter key.
2. Tap Control+Shift+Home to select all texts including newline.
3. Type "ㄱ" again and press space key. (Here you can see that typing "ㄱ" already throws an exception in devtools Proceeding further causes total disruption of the page as described in comment #0.)

With that, the log from mutation observer in Firefox is:

1. type childList: a text node "ㄱ" is added, with nextSibling being a <br>.
2. type characterData: with the target node the same text node in 1.
3. type childList: <br> is removed here.
4. type characterData: with the same target node "ㄱ". It's the same object.
5. type childList: <div> is removed, which is the parent block of the text node, and the target node is it's parent.

1-3 happens on step 1, 4 on step 2, 5 on step 3. With the 5th mutation, draftjs calls findAncestorOffsetKey with the target node, finding data-offset-key attribute in the tree, but the removed parent block is the only block that had it. Thus it fails to find the key and throws.

Chrome also removes div with the same target on step 3, but after step 1 it has two divs, so it still has a div with data-offset-key even when one div is removed, so no exception happens.

Comment 31

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

Sounds like the difference of typing something on this testcase is the root cause. Is it?

Comment 32

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Hmm, some extra information is that at step 1 both Firefox and Chrome gets two divs, but at step 3:

1. Firefox removes all the child divs and replaces it with a text node when typing Korean
   • But not when typing latin alphabets, then it keeps the div 🤔
2. Chrome always keeps the div

Step 1 doesn't have to be Korean, and step 3 applies to Japanese too (except the enter key should be pressed instead), and thus adjusting the title.

Comment 33

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

Well, once I fixed that as not deleting the first block, it seems that it works better. However, even in Chrome, the behavior is odd. So, it's hard to check only by myself. I'll file a bug to change our delete behavior which blocks this bug.

Comment 34

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

The severity field for this bug is set to S3. However, this bug has a P1 WebCompat priority.
:vhilla, could you consider increasing the severity of this web compatibility bug?

For more information, please visit BugBot documentation.

Comment 35

[Jeff Muizelaar [:jrmuizel]]

Masayuki, can you provide on update on the status of this?

Comment 36

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

I fixed some blocker bugs to touch the complicated deletion method which caused new oranges if I touch the method. However, I still don't understand the Chrome's behavior in the case because probably the behavior is not designed one like ours, I mean, the behavior is the result of steps to handle the deletion in some cases. Additionally, I have some bugs which I need to fix as soon as possible. Therefore, I'll be back on bug 1975357 in May.

Comment 37

[Hsin-Yi Tsai (she/her)[:hsinyi]]

Here is my testing result based on Nightly Fx 152.0a1 and 150.01.

In short, I don't see crashes, and I didn't see crashes when I noted in bug 1633399 comment #21 a year ago. With that, I think we need to re-triage this to get the proper priority and impact score assessment. We need to adjust the bug title, which is misleading.

The current difference from the old bug 1633399 comment #21 is - I no longer see all text to be cleared, but I see duplicate texts unexpectedly.

Also note that, I also tried to reproduce with two Chinese IMEs; in my testing, I realized "selecting a new line" is a key; if only text, no new line, is selected , then I get the expected result when typing again. And both of them won't jump to a new line after users press enter after composing a character. The cursor stays at the end of the same line.

Comment 38

[Hsin-Yi Tsai (she/her)[:hsinyi]]

Hi Dennis, can you please re-triage the webcompat priority per comment 37? Thanks.