Ctrl+A selecting text in Slack's "New message" "To:" field and typing appends next characters instead of replacing the selected text
Categories
(Core :: DOM: Editor, defect, P2)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox-esr115 | --- | unaffected |
| firefox120 | --- | unaffected |
| firefox121 | --- | disabled |
| firefox122 | --- | verified |
People
(Reporter: cpeterson, Assigned: masayuki)
References
(Regression)
Details
(Keywords: regression)
Attachments
(2 files)
Masayuki, I bisected this regression to this pushlog for your fix for bug 1851951:
Bug 1851951 landed in Fx 121. I can reproduce this bug in Nightly 121 and 122, but not Beta 121 because the change is behind a pref: editor.block_inline_check.use_computed_style that is only enabled for IS_EARLY_BETA_OR_EARLIER.
Steps to reproduce
- Log into Slack.
- Click Slack's "New message" button.
- Start to type some random characters into the "To:" field.
- Press Ctrl+A to select all the characters you just typed.
- Type some other random characters to replace the selected random characters.
Expected behavior
The new characters should replace the selected characters.
Actual behavior
The new characters are appended to the selected characters. The STR works correctly for other Slack text fields. I can only reproduce this bug in Slack's New message "To:" field.
I have only tested Firefox on Windows 11.
| Assignee | ||
Updated•1 year ago
|
| Assignee | ||
Comment 1•1 year ago
|
||
Hmm, it's really complicated case. The editing host <div> is a flex container. And that has 3 <span>s, first 2 are editable, and the other is not editable. The middle span contains the text which you type. The <span>s may be treated as blocks newly, so, deleting range may have been changed by the new behavior.
On the other hand, while HTMLEditor replaces text, it works as far as checked in the debugger. Therefore, the text was restored by Slack itself. So, I need to check how the legacy mode worked tomorrow.
Updated•1 year ago
|
| Assignee | ||
Comment 2•1 year ago
|
||
Okay, when I type text in the editor the DOM tree becomes:
<div contenteditable style="display:flex">
<span class="c-multi_select_input__space" />
<span class="c-multi_select_input__filter_query">text which I typed</span>
<span class="c-multi_select_input__space" contenteditable="false" />
</div>
In both modes, selection start is adjusted to <span class="c-multi_select_input__filter_query">[text. Then, in the legacy mode, the range is not extended and only the text node and the last <span> are deleted, however, in the new mode, the range is extended and the 2nd and the last <span>s are deleted. Then, Slack restores the content before the deletion. So, I need to think how to behave in this case without breaking the other web apps...
| Assignee | ||
Comment 3•1 year ago
|
||
Hmm, Chrome does same behavior. So, Slack might do something for Chrome...
https://jsfiddle.net/d_toybox/q7ez3bg2/
| Assignee | ||
Comment 4•1 year ago
|
||
| wrong-comment | ||
Oh, I found an odd behavior of Chrome.
https://jsfiddle.net/d_toybox/q7ez3bg2/2/
If there is the empty <span> before the text container, the last non-editable <span> is not removed in Chrome. That might avoid this bug in Slack.
| Assignee | ||
Comment 5•1 year ago
|
||
Comment 2 is wrong. Maybe I was confused between Chrome and Firefox windows.
| Assignee | ||
Comment 6•1 year ago
|
||
Okay, Chrome does not remove the non-editable <span>, however, even if we stopped removing the non-editable node, I still see this bug. So, it seems that Slack does something different (I couldn't align the behavior with changing UA string though).
| Assignee | ||
Comment 7•1 year ago
|
||
Filed a Chromium issue about odd behavior when flex container has a non-editable element.
| Assignee | ||
Comment 8•1 year ago
|
||
Okay, let's forget about the non-editable node. That is not related to us. The root cause is,
<div contenteditable><span>abc</span><span style="display:block">{def</span>}</div> should not delete the 2nd <span> before inserting text. Chrome does not do that.
| Assignee | ||
Comment 9•1 year ago
|
||
The text input of the multi-select combobox of Slack has the following
structure:
<div contenteditable style="display:flex">
<span class="c-multi_select_input__space"> </span>
<span class="c-multi_select_input__filter_query">text which you typed</span>
<span class="c-multi_select_input__space" contenteditable="false"> <span>
</div>
When you do "Select All", they adjust selection to start from start of the
text container <span> (i.e., .c-multi_select_input__filter_query).
Then, typing new character deletes selection first. At this time, in the
legacy mode, AutoDeleteRangesHandler::ExtendOrShrinkRangeToDelete does not
extend the deleting range because <span> is an inline element. However,
in the new mode, it extends the range because the <span> is a block since
it's a flex-item, and selection starts from current block boundary. Then,
deleting range starts before the text container. Finally, Gecko removes the
text container and the following non-editable <span>. Therefore, typing text
will be inserted as a child of the flex container which is the editing host.
Finally, Slack restores the previous structure and collapse selection to end
of the text.
Currently, Chrome does same thing for normal blocks without flex/gird container.
However, doing i in AutoDeleteRangesHandler::ExtendOrShrinkRangeToDelete
causes a lot of regressions. Therefore, this patch tries to avoid only the bug
in Slack. (I think that we need to redesign the deletion handler to fix it.)
Comment 10•1 year ago
|
||
Comment 12•1 year ago
|
||
| bugherder | ||
Updated•1 year ago
|
Comment 14•1 year ago
|
||
I have reproduced this issue with STR from comment 0, on an affected Nightly build (122.0a1, 2023-12-06).
The issue is verified as fixed on Beta 122.0b8 across platforms, Win 11 x64, macOS 13 and Ubuntu 20.04 x64.
Description
•