1205573 - Google Spreadsheets adds extra newline to cells when cutting text and tabbing between cells

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect, P3)

Description

[Chris Peterson [:cpeterson]]

I don't have consistent STR, but I see this problem every few minutes in Firefox, but not once when I spent an hour editing the same spreadsheet in Chrome. I spend a lot of time using Google Spreadsheets, so this problem is pretty annoying. :-) I have seen this problem for a while, so I don't think it is a regression.

When entering a list of items in Google Spreadsheets, I enter the new cell value and press TAB or ENTER to move to the next cell. Occasionally, an extra newline is appended to the cell value (as if I had typed OPTION+ENTER), making the cell too tall.

Comment 1

[Alex Davis [:adavis]]

+1
Happens to me too. It's really annoying!

Comment 2

[Chris Peterson [:cpeterson]]

I have reliable STR. This bug is actually a regression in Firefox 14 (2012)!

@ Masayuki: Is this a problem we are likely to fix? Or should I reach out to Google Spreadsheet developers and ask them work around this Firefox quirk? I can reproduce this bug in Firefox on both Mac and Windows. I cannot reproduce the problem with Chrome, Safari, and Edge.

STR:
1. Select a cell containing text.
2. Press ENTER to start editing the cell in place.
3. Press CMD+X to cut the cell text in place.
4. Press TAB to move to the next cell.

RESULT:
The first cell will gain an extra newline, making the row taller.

Cutting the cell's text is part of the problem. Simply copying the text with CMD+C or typing new text and then pressing TAB does *not* reproduce the newline problem.

I bisected this regression to the following pushlog, which includes editor and execCommand fixes Aryeh made that are likely related:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e29b3d77f808&tochange=f661fb83699b

Comment 3

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

Hmm, I cannot judge if we should which choice for now. I'd like to see minimized testcase for this. Can somebody create it? (Do we have such team?)

Comment 4

[Astley Chen (inactive)]

I've a simpler STR to reproduce the issue:

STR:
1. Pick up a cell and enter 2 lines of text in it.
2. Double click 2nd line or just simply delete 2nd line words by chars until all chars are deleted.

RESULT:
*After step 2, cursor will move back to the end of 1st line* and 
the first cell will gain an extra newline, making the row taller.

AFAIK, in general html textarea, the same steps giving different result on cursor position(stay in 2nd line). In GSheet, apparently there is an existing CR/LF char in the end of 1st line so that text metrics is given a 2 lines text in the cell, however, cursor's gone back to 1st line which is incorrect and misleading user.
Hope this info helps although I'm still unable to simply it into simplified test cases.
BTW, GSheet currently is using Canvas2D to draw text and measure text.

Comment 5

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

I'm not really sure how to begin debugging this.  If someone could give a more precise regression range than in comment 2, ideally one commit, that might help.  I can reproduce with the STR in comment 2.

Comment 6

[Alice0775 White]

Last Good: 
http://hg.mozilla.org/projects/birch/rev/63b24c3beddc
Mozilla/5.0 (Windows NT 6.2; WOW64; rv:14.0) Gecko/20120423 Firefox/14.0a1 ID:20120423040206

First Bad:
http://hg.mozilla.org/projects/birch/rev/b20ee61f3aed
Mozilla/5.0 (Windows NT 6.2; WOW64; rv:14.0) Gecko/20120424 Firefox/14.0a1 ID:20120424040230

Suspect: Bug 599983 - Remove use or optimize performance of moz attributes in editor and serializer

Comment 7

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Thanks, Alice!  That is extremely interesting.  I *think* all that commit does is inhibit the output of moz_dirty attributes, which we never used except for serializing.  So it seems hard to see how it could change anything here, unless Google had some special case that they wrote more than 4.5 years ago that did something different based on moz_dirty.  It seems unlikely, but I don't know of any good way to search the sources of the JS on the page to look for it.  Or maybe the commit had some sort of side effect that I'm not spotting?

The STR in comment #2 don't work consistently for me, although they do work a lot of the time.  (Hopefully the bisect was correct and wasn't just because the STR aren't completely consistent for Alice either?)  I don't understand the STR in comment #4.  If someone could get me STR that work completely consistently, maybe I could take a look at some point, although I'm scared to try debugging Google's JavaScript!  The STR should start with "Start a new Google Spreadsheet" and give precise instructions as to every click and keystroke.

Comment 8

[Alice0775 White]

Reproducible: 100%

Steps to reproduce:
1. Open a Sheet and Double click on a cell to activate to edit directly in the cell
2. Type some text in the cell and press [Enter]
3. Double click on the cell to activate to edit directly in the cell. 
4. Press Ctrl+A and then Ctrl+X
5. Press [Tab] key or click the other cell to move active cell

Comment 9

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Thanks.  I took a brief look, but their code is completely obfuscated, and I'm not willing to try debugging it.  Has anyone reported this issue to Google?  Is there any way to report this to them?

Comment 10

[Desigan Chinniah [:cyberdees] [:dees] [London - GMT]]

(In reply to Aryeh Gregor (:ayg) (working until November 1) from comment #9)
> Thanks.  I took a brief look, but their code is completely obfuscated, and
> I'm not willing to try debugging it.  Has anyone reported this issue to
> Google?  Is there any way to report this to them?

+Steven - who could direct this to Sheets engineers

Comment 11

[Steven Saviano]

Thanks for the info!

I filled bug # 32553590 on Google-side. An eng is expected to pick it up within a week.

Comment 12

[Josh Danziger]

Just a quick update from the Google-side.  It turns out that we were relying on the moz_dirty bits as suggested in comment #7 and are working on a prospective fix.

Comment 13

[Steven Saviano]

Update: This fix should be rolling out this week on the Google Docs side. Will mark the bug as resolved once it is complete.

Comment 14

[Steven Saviano]

Apologies for delay. This bug was rolled out a number of weeks ago.

Please feel free to mark as fixed and verify as appropriate.

Comment 15

[Steven Saviano]

Correction: The fix was rolled out a while ago (not the bug :) )

Comment 16

[Chris Peterson [:cpeterson]]

Thanks! I verified this bug is fixed in Firefox 52 and Nightly 54.