1881906 - Typing a space on last position of a contenteditable injects a br - behaviour changed on last release

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[iguypouf]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:123.0) Gecko/20100101 Firefox/123.0

Steps to reproduce:

In a contenteditable table cell, i press the spacebar at the end of the text

Actual results:

a line-break ( <br> ) is added instead below the text, and the next letter appears pasted to the previous text, so its impossible to separate words

Expected results:

a space appears like in webkit, or like the behaviour before the release of FF123.

Comment 1

[Andreas Farre [:farre]]

Attachment: Contenteditable table cell

Thank you for your report!

I made a testcase out of the description, but I couldn't reproduce it, neither on the release channel or the latest nightly. Does the test case correspond to what you're seeing?

Comment 2

[iguypouf]

Attachment: BrBug.html

Smallest Reproductible bug

Comment 3

[iguypouf]

Hi Andreas, thanks to taking care of that.
I was hoping that the new release changelog ( internally ) would bring you directly to the problem; as its not the case I took the time to deconstruct the dom of my app and identified that the most reasonable cause is the "display" rule on "br" tag.
This rule may seems weird but we really need it in our app ( a complete web based document editor ), and this always worked before the release of FF123, or maybe one or two previous version if the user who raised that jumped several releases.
Thank you again,

Comment 4

[Daniel Moreira]

Attachment: 2024-02-28_09h38_57.mp4

Hi Andrea,

I confirm that we have the same issue : we have a searchbar on our web app and when typing spaces, those are removed if I add another character.

Not happening in FF 122.0 but only in FF 123.0, I've made a small GIF to show the problem.

Everytime I type "asdf", I also type 4-5 spaces and you can see that only one space shows up. If I continue typing "asdf", the previous space is removed.

Comment 6

[Andreas Farre [:farre]]

I found a regression range and it appears that this was introduced by Bug 1851951. Masayuki, since you wrote the patch for Bug 1851951, do you have an idea?

Comment 7

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

I'll take a look.

Comment 8

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

Hmm, attachment 9386818 [details] behaves odd for me. I can put caret after a white space at end of the line/cell. However, cannot select it with non-collapsed selection range. I believe that the latter behavior is right since the white spaces at end of the table cell (i.e., block) should be collapsed and invisible. Therefore, our editor considers that typing text won't show invisible white space between new character and the last character before the invisible white space. So the problem here is, why the white space is visible with collapsed selection. It seems that nsTextFrame::PeekOffsetCharacter works TextRun, so I guess TextRun does not care of invisible white spaces around a block boundary.

Is this expexted behavior for nsTextFrame?

Comment 9

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

So I think that if you type a character into the test case, deleting the white space may be exoected behavior from the editor module of view, however, once you type a white space and then type a character, the space should not be removed. I.e., the later is at least a bug of the editor. That should be caused by the block <br> handling .

Comment 10

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

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

Comment 11

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

Attachment: Bug 1881906 - Make `HTMLEditUtils` treat `<br>` elements always inline r=m_kato!,#dom-core

As far as I've tested, <br> element is always treated as usual even if its
display is set to anything except none. Therefore, preceding collapsible
white-spaces and <br> element should be treated as visible even if the
following <br> element is display:block or something.

Note that if display:none is specified, we should not treat it as a line
break, but our editor cannot work properly in the case and HTMLEditUtils
currently uses the default style of HTML elements if display:none is
specified. Therefore, this patch makes HTMLEditUtils always treat <br>
as inline.

Comment 12

[Jonathan Kew [:jfkthame]]

This is interesting, actually....it turns out I can even select the trailing space in a block in a minimal example like:

data:text/html,<div>Select space after this: </div>

without involving contentEditable at all. This seems surprising (and differs from Chrome/Safari), and may be a bug, but I guess this has been our behavior for a long time. I'm not sure what the impact of attempting to change it might be. There are a lot of subtle interactions involving space-trimming/collapsing behavior, so we need to tread carefully.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b3c409d3ff5a
Make `HTMLEditUtils` treat `<br>` elements always inline r=m_kato

Comment 14

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

(In reply to Jonathan Kew [:jfkthame] from comment #12)

This is interesting, actually....it turns out I can even select the trailing space in a block in a minimal example like:

data:text/html,<div>Select space after this: </div>

without involving contentEditable at all.

Oh, right. It's odd.

This seems surprising (and differs from Chrome/Safari), and may be a bug, but I guess this has been our behavior for a long time. I'm not sure what the impact of attempting to change it might be. There are a lot of subtle interactions involving space-trimming/collapsing behavior, so we need to tread carefully.

Thanks, for now, I just partially backout the behavior change for <br> in the editor module.

Comment 15

[Cristian Tuns]

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

Comment 16

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

Comment on attachment 9388987 [details]
Bug 1881906 - Make HTMLEditUtils treat <br> elements always inline r=m_kato!,#dom-core

Beta/Release Uplift Approval Request

• User impact if declined: User cannot type text written in languages whose words are separated by an ASCII white-space if <br> element is set to display:block.
• 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: Low
• Why is the change risky/not risky? (and alternatives if risky): Partially backout the regressor only for <br> element.
• String changes made/needed:
• Is Android affected?: Yes

Comment 17

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

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

Comment 18

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

Upstream PR merged by moz-wptsync-bot

Comment 19

[Dianna Smith [:diannaS] - on Leave, back on August 9th]

Comment on attachment 9388987 [details]
Bug 1881906 - Make HTMLEditUtils treat <br> elements always inline r=m_kato!,#dom-core

Approved for 124.0b9

Comment 20

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/542f336d1464