Closed Bug 1738134 Opened 8 months ago Closed 5 months ago

Google Sheets ignores first Ctrl+Enter newline when entering text in a cell

Categories

(Web Compatibility :: Desktop, defect)

defect

Tracking

(Webcompat Priority:P1, firefox-esr78 unaffected, firefox-esr91 unaffected, firefox93 unaffected, firefox94- wontfix, firefox95- wontfix, firefox96 affected)

RESOLVED FIXED
Webcompat Priority P1
Tracking Status
firefox-esr78 --- unaffected
firefox-esr91 --- unaffected
firefox93 --- unaffected
firefox94 - wontfix
firefox95 - wontfix
firefox96 --- affected

People

(Reporter: cpeterson, Unassigned)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression, webcompat:site-wait)

Attachments

(1 file, 1 obsolete file)

[Tracking Requested - why for this release]:

Masayuki, this is a regression in Firefox 94 from HTMLEditUtils::IsVisibleBRElement() bug 1727868. I bisected this regression to this pushlog:

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=f3a453eb5700042249820b9cb060492c64e5a5c2&tochange=0c79c667c59a344d54f360ffe67b92823790b6e7

Steps to reproduce

  1. Create a new Google Spreadsheet: https://docs.google.com/spreadsheets/
  2. In a cell, type a string like a.
  3. While still editing the cell, press Ctrl+Enter to insert a newline in the cell and type a string like b.
  4. Then press Ctrl+Enter again to insert a newline in the cell and type a string like c.

Expected result

The cell should have three lines that look like:

a
b
c

Actual result

The cell has only two lines and looks like:

ab
c
Flags: needinfo?(masayuki)

Probably not going to fix this in 94.x, but I'd be open to a dot release ride along if there's a simple, low-risk patch available.

Cannot reproduce this with minimal testcase:

data:text/html,<div contenteditable style="white-space:pre-wrap"><span>a%0A<br></span></div>

nor simple code to implement it: https://jsfiddle.net/d_toybox/1r6ae52o/

, and Ctrl + Enter isn't standard key assign of Gecko to insert a line break. So this could be caused by their script.

Flags: needinfo?(masayuki)

Er, no. It's indeed a bug of the method. The innerHTML at typing Ctrl + Enter is:

a<br><span style="font-size:13px;color:#000000;font-weight:normal;text-decoration:none;font-family:'Arial';font-style:normal;text-decoration-skip-ink:none;"></span>

It seems that the <br> element is considered as visible due to followed by the <span>. Keep investigating...

Assignee: nobody → masayuki
Status: NEW → ASSIGNED

Previously, it used WSRunScanner::ScanNextVisibleNodeOrBlockBoundary() looking for a following block boundary or visible node.

It treats inline elements as visible even if actually not so.

Now, the method works differently, it ignores all inline elements except replaced elements.

But the new behavior matches with actual layout result. So, it fixed* the mismatch...

Hmm, I wonder, the <span> in Google Sheets is visible (width: 0px, height: 15px) so that the preceding <br> element is visible. Why?? I don't see any style to make it visible...

FWIW,
Login to Google Sheets.
Then Spoof the UA to "Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/95.0.4638.54 Safari/537.36" and reload the spreadsheet.
Then the Ctrl+Enter works as expected.

(In reply to Alice0775 White from comment #6)

FWIW,
Login to Google Sheets.
Then Spoof the UA to "Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/95.0.4638.54 Safari/537.36" and reload the spreadsheet.
Then the Ctrl+Enter works as expected.

Thank you, Alice-san, the cell is

<div
  class="cell-input editable"
  tabindex="0"
  role="combobox"
  docs-unhandledkeys="ctrl+shift+z,ctrl+y,ctrl+z"
  id="waffle-rich-text-editor"
  style="background-color: rgb(255, 255, 255);
         font-size: 13px;
         color: rgb(0, 0, 0);
         font-weight: normal;
         font-family: &quot;Arial&quot;;
         font-style: normal;
         text-decoration-skip-ink: none;"
  dir="ltr"
  g_editable="true"
  aria-autocomplete="list"
  aria-label="C6"
  contenteditable="true"
>a<br><span style="font-size:13px;
                   color:#000000;
                   font-weight:normal;
                   text-decoration:none;
                   font-family:'Arial';
                   font-style:normal;
                   text-decoration-skip-ink:none;"></span></div>

And we had not supported white-space:pre-wrap in editor since bug 1724650. So that I guess that they use a specific path for Gecko and that depends on some bugs of Gecko (our editor does not refer CSS in most cases, that makes web developers confused like checking the <br> element is visible or not).

The reason why the text is inserted before the <br> element is, editor adjusts caret before invisible <br> element before inserting text.

However, if I write WPT with data:text/html,<div contenteditable style="white-space:pre-wrap">a<br><span style="padding:1px"></span></div>, Chrome also inserts new text before the <br> element. So, restoring the original behavior restores incompatible behavior too. So we should not change the behavior for the other web apps.

Assignee: masayuki → nobody
Status: ASSIGNED → NEW
Component: DOM: Editor → Desktop
Product: Core → Web Compatibility

I wonder if the webcompat team can fix this with a site intervention.
Let's try to contact first Google.

Google has been contacted.

Webcompat Priority: --- → P1

If a <br> element follows a visible thing and there is no visible content
between it and following block boundary (both child's and parent's), it's
called as "invisible" BR element at Mozilla.

Selection API can put caret after it but before the following block boundary
and of course, user may be able to put caret at there if the browser has a
bug like https://bugzilla.mozilla.org/show_bug.cgi?id=1738193.

Even in this case, user can type something if it's in editable content, and
even though this is an edge case, it's safe to get same result in any browsers
since web apps may put caret there accidentally.

The expected results are based on Blink's behavior and Gecko fails the cases
if the following block boundary is of a child block. However, no patch for
Gecko for now.

Assignee: nobody → masayuki
Status: NEW → ASSIGNED

Sorry for the bug spam due to the wrong bug#.

Assignee: masayuki → nobody
Status: ASSIGNED → NEW

Comment on attachment 9248812 [details]
Bug 1738134 - Add new tests to check the behavior of insertText command when caret is after "invisible" <br> element r=m_kato!

Revision D130137 was moved to bug 1738200. Setting attachment 9248812 [details] to obsolete.

Attachment #9248812 - Attachment is obsolete: true

This is getting late in the cycle for a potential fix to be uplifted to beta, marking as wontfix for 95.

Masayuki-san,
Is it an actionable bug for Core? Or is it just an issue where you would like the webcompat team to do outreach to the site.

Flags: needinfo?(masayuki)

(In reply to Karl Dubost💡 :karlcow from comment #15)

Masayuki-san,
Is it an actionable bug for Core? Or is it just an issue where you would like the webcompat team to do outreach to the site.

See comment 8, I think that we can do nothing in Core.

Flags: needinfo?(masayuki)
Attached image current-rendering

Thanks Masayuki-san.

On December 2021, on the list, Google said:

We've identified a fix, and it should reach production within a few weeks. There was a Firefox-specific hack that became no longer necessary in some recent build, so we just removed the hack.

Let's test if it has been fixed.

And yes this has been fixed. We can close here.

Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.