1839660 - Discord editor sometimes mis-places cursors when typing words

Status: RESOLVED INVALID

(Core :: DOM: Selection, defect)

Description

[Dennis Schubert [:denschub]]

Attachment: discord-input-bug.mp4

STR:

1. Open Discord, any channel.
2. Type a message with lots of words - or just mash some keys and the spacebar.

Sometimes, after entering a word, the cursor is placed one or more positions too far to the left, i.e. before the end of the previous word. Continuing to type then inserts characters between a previous word. I've attached a screen recording where this issue is reproduced multiple times.

Note that this is somewhat intermittent. Sometimes, I can reproduce this easily, and sometimes, it takes me a lot of seconds to get this to break. :/

I can't reproduce this in a blank contenteditable, so that's something in Discord's JavaScript. I have not tried to diagnose this, but if you want some help digging through their JS, I'm happy to help.

Comment 1

[Dennis Schubert [:denschub]]

mozregression says that this is a regression from bug 1817723. Sean, can you have a look and try to figure out what's happening here?

Comment 2

[Dennis Schubert [:denschub]]

[Tracking Requested - why for this release]:

Requesting tracking just to put this regression on people's dashboards. This regression can make typing in Discord rather infuriating (see the attached screen recording). We can try reaching out to Discord if we think this is a bug on their end, but at this point, I don't know if this affects other applications as well.

Comment 3

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

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

Comment 4

[Sean Feng [:sefeng211]]

Thanks, looking

Comment 5

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

The bug is marked as tracked for firefox116 (nightly). We have limited time to fix this, the soft freeze is in 7 days. However, the bug still isn't assigned and has low severity.

:hsinyi, could you please find an assignee and increase the severity for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 6

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

Bumping up to S2 given this is a recent regression, impacting a popular site.

Comment 7

[Andrew McCreight [:mccr8]]

I've also noticed this. I haven't seen similar issues on any other sites with text boxes, like Slack, Element, Bugzilla or Gmail.

Comment 8

[Sean Feng [:sefeng211]]

I am still looking, these are my findings so far.

I think how Discord's input box works is, the input box is a contenteditable div and they listen the beforeinput event to catch which key the user has pressed. They also use preventDefault() to not letting the browser updates the input box, instead they'll use editor.textContent to update the value manually. And then they'll use some other javascript to manually update the cursor position.

Perhaps some of the changes caused some confusions to their cursor position updating code. This is what I am still investigating.

Comment 9

[Sean Feng [:sefeng211]]

I'll ask to back out the patch if I can't figure this out before the soft release.

Comment 10

[Dustin L. Howett]

I was just writing up a parallel submission to this one, as mozregression pointed me to D178215. Symptoms are the same, cause is the same, so I'm going to avoid polluting the bug tracker with another report. :)

Comment 11

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

When I try to reproduce this bug, I reproduce this mostly when it takes time a sec. So it seems that there is a race condition between the tree updater and selection setter. According to the Selection API log, only Selection.setBaseAndExtent is used by the web app (I guess it's here). If it's in slate, it may be called in a callback of requestAnimationFrame... I still don't understand why the patch affects this because the all event listeners of our builtin editor runs after all event listeners in content.

I wonder, it may be safer to cancel changing the event listeners' phase and improve IsAcceptableInputEvent instead.

Comment 12

[Sean Feng [:sefeng211]]

This patch that caused the regression has been backed out, so I am changing the tracking flag for that.

Comment 13

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

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

Comment 14

[Sean Feng [:sefeng211]]

The bug that caused this regression has been relanded with a fix for this. And I've verified this is fixed in the latest Nightly, so closing this.