Splitting Gmail rich-text list with Enter misplaces the caret

VERIFIED FIXED in Firefox 8

Status

()

Core
Editor
--
major
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: Nickolay_Ponomarev, Assigned: kaze)

Tracking

({regression})

Trunk
mozilla9
regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox5 unaffected, firefox6 unaffected, firefox7 unaffected, firefox8+ fixed, firefox9 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
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.)
(Reporter)

Updated

6 years ago
tracking-firefox8: --- → ?
Assignee: nobody → kaze
(Assignee)

Comment 1

6 years ago
Hmm, I hope it’s not a regression caused by my patch on bug 674861. Looking at it.
(Assignee)

Comment 2

6 years ago
Created attachment 554354 [details]
testcase
(Assignee)

Comment 3

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

Thanks for your report!
status-firefox5: --- → unaffected
status-firefox6: --- → unaffected
status-firefox7: --- → unaffected
status-firefox8: --- → affected
status-firefox9: --- → affected
(Assignee)

Comment 4

6 years ago
Created attachment 554393 [details] [diff] [review]
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?
Attachment #554393 - Flags: review?(ehsan)
(Assignee)

Updated

6 years ago
Status: NEW → ASSIGNED
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Comment 5

6 years ago
http://tbpl.allizom.org/?usebuildbot=1&tree=Try&rev=31d671f9f0f7
(Assignee)

Comment 6

6 years ago
Ugh, tests failing. Looking at it. :-(
Comment on attachment 554393 [details] [diff] [review]
patch proposal

Can you please re-request the review when you fix the test failures?
Attachment #554393 - Flags: review?(ehsan)
(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?
(Assignee)

Comment 9

6 years ago
Created attachment 554553 [details] [diff] [review]
patch proposal

unit tests now work properly
Attachment #554393 - Attachment is obsolete: true
Attachment #554553 - Flags: review?(ehsan)
(Assignee)

Comment 10

6 years ago
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.
Attachment #554558 - Flags: approval-mozilla-aurora?
Attachment #554553 - Flags: review?(ehsan) → review+
(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!
Pushed to inbound.
http://hg.mozilla.org/mozilla-central/rev/d6745c14f051
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
(Reporter)

Comment 14

6 years ago
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
Status: RESOLVED → VERIFIED
Comment on attachment 554558 [details] [diff] [review]
backout - bug674861

Approved for Aurora (Update 8.)  Please land this as soon as possible, thanks!
Attachment #554558 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Bug 674861 backed out from Aurora: http://hg.mozilla.org/releases/mozilla-aurora/rev/b71f847ed644
status-firefox8: affected → fixed
status-firefox9: affected → fixed

Comment 17

6 years ago
---------------------------------[ Triage Comment ]---------------------------------

Even though this is fixed we'll track it to make sure it doesn't bounce.
tracking-firefox8: ? → +
You need to log in before you can comment on or make changes to this bug.