1940278 - Space character disappears from Google Calendar description, when typing after deleting the last word

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Daniel Holbert [:dholbert]]

This bug basically manifests in the same way that bug 1925635 does, but it's specific to Google Calendar (in my experience so far at least) and it was regressed by the patches that landed in bug 1925635 [EDIT: see comment 2]

STR:

1. Visit https://calendar.google.com/ and sign in if you're not already signed in. (I'm using a personal Google account.)
2. Click a blank space on your calendar to create an event, and then click "More Options"
3. Click the description field ("Add description")
4. Type abc def
5. Ctrl+Backspace (or option+backspace on macOS) to delete def
6. Type g

EXPECTED RESULTS:
Description should show abc g (preserving the space)

ACTUAL RESULTS:
Description shows abcg (space gets eaten when I type the g)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: screencast of bug

Comment 2

[Daniel Holbert [:dholbert]]

mozregression pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=855890e2cd16cf21ac6f740b83aa69ab2519b1b9&tochange=ff23e6f39ed6d08a39526e40120d16e623dfa830

Before that range, I get expected results.

When writing the original description (comment 0), I had skimmed that pushlog and thought the commits were all from bug 1925635 and flagged this as a regression from that -- but looking more closely, I see bug 1923251 in there too ("clean up unnecessary padding line breaks") and I think this is more likely a regression from that.
--> Regression from bug 1923251

Comment 3

[Daniel Holbert [:dholbert]]

(note, the screencast also shows some unrelated graphical glitches around t=3s, just after I click "More options" -- there are two bits of UI that are superimposed near the top left, as the more-options UI comes up. Avoid the temptation to dive into that issue; I already took a look at that a few weeks ago, and I confirmed that it happens in Chrome too, so that part is not our bug.)

Comment 4

[Daniel Holbert [:dholbert]]

[Tracking Requested - why for this release]: recent regression affecting a prominent site (Google Calendar); would be nice to fix in v135 before shipping to release.

Comment 5

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

This is reproducible with normal Backspace. According to the debugger, the padding <br> is intentionally removed by the Calendar. So, basically, this is a bug of Google. I guess the reason why this is a regression is, they implement their on undo stack. For implementing that, it might store the previous DOM tree before the deletion when there is no padding <br> in the newer build, but there is a padding <br> in the older build.

For now, I think we should replace the collapsible white-space when padding <br> is removed by the web app until bug 503838 gets fixed.

Comment 6

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

Attachment: Bug 1940278 - Make `HTMLEditor` replace collapsible ASCII white-space with an NBSP if it's visible by a following <br> but it's removed r=m_kato!

This is a hack until bug 503838 is fixed.

The other browsers puts an NBSP for the last ASCII white-space immediately
before a block boundary to make it visible. However, we currently need to
keep using an ASCII white-space as-is with putting a padding <br> element.

However, web apps may delete the <br> for some reasons without maintaining
the preceding collapsible white-space visibility since it's not required in
the other browsers.

Therefore, this patch replaces the white-spaces only when we meet such
situation. Of course, this breaks the undo stack, but touching the DOM anyway
causes breaking the undo stack. Thus, we can trust the web app as that it
manages their on undo stack.

Comment 7

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

The bug is marked as tracked for firefox135 (beta) and tracked for firefox136 (nightly). However, the bug still has low severity.

:hsinyi, could you please increase the severity for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 8

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

Attachment: Bug 1940278 - Make `HTMLEditor` replace collapsible ASCII white-space with an NBSP if it's visible by a following <br> but it's removed r=m_kato!

This is a hack until bug 503838 is fixed.

The other browsers puts an NBSP for the last ASCII white-space immediately
before a block boundary to make it visible. However, we currently need to
keep using an ASCII white-space as-is with putting a padding <br> element.

However, web apps may delete the <br> for some reasons without maintaining
the preceding collapsible white-space visibility since it's not required in
the other browsers.

Therefore, this patch replaces the white-spaces only when we meet such
situation. Of course, this breaks the undo stack, but touching the DOM anyway
causes breaking the undo stack. Thus, we can trust the web app as that it
manages their on undo stack.

Original Revision: https://phabricator.services.mozilla.com/D233472

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/020e3aecfaaa
Make `HTMLEditor` replace collapsible ASCII white-space with an NBSP if it's visible by a following <br> but it's removed r=m_kato

Comment 10

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

The change is a little bit risky for the other web apps. If the change breaks some other web apps, we should limit the new behavior only for Google Calendar. I think that it's possible with referring the <body> tag attribute. See my comment in Phabricator for the detail.

Comment 11

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

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

Comment 12

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/020e3aecfaaa

Comment 13

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/bc06489cb97d

Comment 14

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

Upstream PR merged by moz-wptsync-bot