1852849 - Composing message, turning on italics, underline or bold jumps to line start

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[MoisheP]

Steps to reproduce:

1. Start new message or reply.
2. Enter a line of text with multiple words.
3. On second or subsequent line, type more words and then enter italic, underline or bold mode either by clicking the icon or pressing Ctrl-i, -i or -b.
4. Continue typing.

Actual results:

1. Text entry jumps to beginning of line. Caret, however, stays at end of line until more text is entered.
2. Text attributes are unchanged.

Expected results:

1. Text should appear on the same line at caret, after last character entered.
2. Character attributes should change to bold, italic or underline.

Comment 1

[betterbird.project+10]

We see this, too. Cannot be reproduced in FF Daily at https://www-archive.mozilla.org/editor/midasdemo/, so maybe TB specific.

Alice, can you please find the regression range for Thunderbird. Works correctly in TB 102, but not 115. Thanks in advance!

Comment 2

[Alice0775 White]

I cannot reproduce the issue on 115.2.2esr and Daily119.0a1 Windows10.

Comment 3

[betterbird.project+10]

Yes, it seems that if you compose with default settings, that is, variable width, the issue doesn't occur. So select a font or "fixed width" in the compose setting so or do this:

Start compose window, switch font to Times, type a word, hit enter, in the second paragraph type another word, then Ctrl+B or click the "B", continue typing.

The error doesn't seem to appear on the first line.

Comment 4

[betterbird.project+10]

Actually, it happens in Firefox Daily at https://www-archive.mozilla.org/editor/midasdemo/.

Switch font to Arial , type a word, hit enter, in the second line type another word, click the "B", continue typing.

Please move the but to Core::Editor.

Comment 5

[Alice0775 White]

(In reply to betterbird.project+10 from comment #4)

Actually, it happens in Firefox Daily at https://www-archive.mozilla.org/editor/midasdemo/.

Switch font to Arial , type a word, hit enter, in the second line type another word, click the "B", continue typing.

Please move the but to Core::Editor.

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a7465d2d89badae7c4e3eba379d03a91c8a431b6&tochange=5bacd687fa98b0e9a5dea9944f360fdc5a6770e6

Comment 6

[Alice0775 White]

[Tracking Requested - why for this release]: text editor styling and insertion point
is broken.

Comment 7

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

:masayuki, since you are the author of the regressor, bug 1807829, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

I'll be back after finishing my current work.

Comment 9

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

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

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Attachment: Bug 1852849 - Make `HTMLEditor::AutoInlineStyleSetter::ExtendOrShrinkRangeToApplyTheStyle` recompute common ancestor of the range if it updates the range r=m_kato!

In this case, <font face="monospace">abc []<br></font>, the range applying
new style will be extended as <font face="monospace">abc [<br>}</font> first.
https://searchfox.org/mozilla-central/rev/29bdf6ff9965a647c6f64d63fed2b5bd094532c7/editor/libeditor/HTMLStyleEditor.cpp#1944

Then, the start point should be shrunken and the range should become
<font face="monospace">abc {<br>}</font>.
https://searchfox.org/mozilla-central/rev/29bdf6ff9965a647c6f64d63fed2b5bd094532c7/editor/libeditor/HTMLStyleEditor.cpp#1977-1978

However, commonAncestor is still the text node because it's not updated
after extending the range to include the <br>. Then,
AutoInlineStyleSetter::GetNextEditableInlineContent fails to get <br>
from the text node.
https://searchfox.org/mozilla-central/rev/29bdf6ff9965a647c6f64d63fed2b5bd094532c7/editor/libeditor/HTMLStyleEditor.cpp#1576-1578

Finally, the unexpected range computation will reach here with the text editor
and adjust the start of the range to start of the text node.
https://searchfox.org/mozilla-central/rev/29bdf6ff9965a647c6f64d63fed2b5bd094532c7/editor/libeditor/HTMLStyleEditor.cpp#420-423

Therefore, the new text which the new style should be applied is jumped to
start of the text node.

Comment 12

[Allen]

Related buggy behavior:

Set font to a named font (e.g., Palatino Linotype). Type a few words. Hit return.
Set font to Variable. Type a few words.
First char typed is consumed and does not appear.

Example:

1234 1234 (in Palatino)
(switch font to Variable Width, then type same string:)
234 1234

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c9c063011de3
Make `HTMLEditor::AutoInlineStyleSetter::ExtendOrShrinkRangeToApplyTheStyle` recompute common ancestor of the range if it updates the range r=m_kato

Comment 14

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

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

Comment 15

[Noemi Erli[:noemi_erli]]

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

Comment 16

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

Upstream PR merged by moz-wptsync-bot

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

The patch is cleanly graftable to ESR 115. I'll request the uplift.

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Comment on attachment 9356563 [details]
Bug 1852849 - Make HTMLEditor::AutoInlineStyleSetter::ExtendOrShrinkRangeToApplyTheStyle recompute common ancestor of the range if it updates the range r=m_kato!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: new regression in 111, and this may affect actual editable web apps in the wild.
• User impact if declined: While typing a text with multiple styles, caret may be jumped to unexpected position and really annoying to type text.
• Fix Landed on Version: 120
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Just adding the recomputing code (one line fix if ignore the error handling).

Comment 19

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

Comment on attachment 9356563 [details]
Bug 1852849 - Make HTMLEditor::AutoInlineStyleSetter::ExtendOrShrinkRangeToApplyTheStyle recompute common ancestor of the range if it updates the range r=m_kato!

Approved for 115.5esr.

Comment 20

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/03f4ed2f206d

Comment 21

[Bogdan Maris, Desktop QA]

Reproduced using an old Nightly from 2023-09-11 using the demo page from comment 4, verified that using latest Nightly 121.0a1, Firefox Beta 120.0b1 and Firefox ESR build (https://treeherder.mozilla.org/jobs?repo=mozilla-esr115&revision=c900bad131294eb3fdb29138411e480fe27e0fd7) this is no longer an issue. Testing was done across platforms (Windows 10, macOS 13 and Ubuntu 22.04). I will not change the status of this bug to Verified yet since it should also be verified on Thunderbird before doing so.