680279 - Splitting Gmail rich-text list with Enter misplaces the caret

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Nickolay_Ponomarev]

Sorry for the Gmail testcase, I don't have the time to attempt to reduce it.

1. Log in https://mail.google.com/mail/?shva=1#inbox (the desktop 
2. click Compose
3. Create an unordered list by clicking the respective icon
4. type x, Enter, x, Enter, x, creating three list items with the text "x"
5. Position the caret at the end of the first item and press Enter twice

Expected results (roughly):
  <ul><li>x</li></ul>
  <p>{caret}</p>
  <ul>..the rest of the list</ul>

Actual results:
  <ul><li>x</li></ul>
  <ul><li>x</li><li>x</li></ul>
  <p>{caret}</p>

Works: 2011-08-03-03-07-53-mozilla-central.app
Fails: 2011-08-04-03-07-32-mozilla-central.app
and still on Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0a1) Gecko/20110818 Firefox/9.0a1

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3735fb1cd5ef&tochange=be4b064f1159

Bug 674861 is the only obviously related item I could find, although I understand it was supposed to change the behaviour in a different case.

(When I initially tried to reproduce on a clean Firefox profile, the issue was reproducible with one account that uses the Gmail "Preview (dense)" theme and was not reproducible with another account. Now I can reproduce with any account.)

Comment 1

[Fabien Cazenave [:kaze]]

Hmm, I hope it’s not a regression caused by my patch on bug 674861. Looking at it.

Comment 2

[Fabien Cazenave [:kaze]]

Attachment: testcase

Comment 3

[Fabien Cazenave [:kaze]]

Found the regression, rushing to submit a patch today so it can be shipped with Firefox 8.

Thanks for your report!

Comment 4

[Fabien Cazenave [:kaze]]

Attachment: patch proposal

Fixes the regression introduced in bug 674861.
New tests have been added to make sure that lists are properly splitted (i.e. that a paragraph is created at the right place) when allowed.

Ehsan, is it still time to get this patch in Firefox 8?

Comment 5

[Fabien Cazenave [:kaze]]

http://tbpl.allizom.org/?usebuildbot=1&tree=Try&rev=31d671f9f0f7

Comment 6

[Fabien Cazenave [:kaze]]

Ugh, tests failing. Looking at it. :-(

Comment 7

[(no longer active)]

Comment on attachment 554393 [details] [diff] [review]
patch proposal

Can you please re-request the review when you fix the test failures?

Comment 8

[(no longer active)]

(In reply to Fabien Cazenave (:kazé) from comment #4)
> Ehsan, is it still time to get this patch in Firefox 8?

I would really like to fix this in Firefox 8 by backing out bug 674861.  Can you please attach a patch which reverts bug 674861 on mozilla-aurora and ask approval-aurora on it?

Comment 9

[Fabien Cazenave [:kaze]]

Attachment: patch proposal

unit tests now work properly

Comment 10

[Fabien Cazenave [:kaze]]

Attachment: backout - bug674861

For Firefox 8, the two following patches should be applied in this order:
 • attachment 550096 [details] [diff] [review]
 • attachment 554553 [details] [diff] [review] 

Or, I can make a single patch instead of these two ones. Let me know.

Comment 11

[(no longer active)]

(In reply to Fabien Cazenave (:kazé) from comment #10)
> Created attachment 554558 [details] [diff] [review]
> backout - bug674861
> 
> For Firefox 8, the two following patches should be applied in this order:
>  • attachment 550096 [details] [diff] [review]
>  • attachment 554553 [details] [diff] [review] 
> 
> Or, I can make a single patch instead of these two ones. Let me know.

No, this should be fine.  Thanks!

Comment 12

[(no longer active)]

Pushed to inbound.

Comment 13

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

http://hg.mozilla.org/mozilla-central/rev/d6745c14f051

Comment 14

[Nickolay_Ponomarev]

Thanks! Fixed for me as well on Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0a1) Gecko/20110822 Firefox/9.0a1

Comment 15

[Christopher Blizzard (:blizzard)]

Comment on attachment 554558 [details] [diff] [review]
backout - bug674861

Approved for Aurora (Update 8.)  Please land this as soon as possible, thanks!

Comment 16

[(no longer active)]

Bug 674861 backed out from Aurora: http://hg.mozilla.org/releases/mozilla-aurora/rev/b71f847ed644

Comment 17

[LegNeato]

---------------------------------[ Triage Comment ]---------------------------------

Even though this is fixed we'll track it to make sure it doesn't bounce.