Last Comment Bug 674861 - contentEditable lists should not be splittable
: contentEditable lists should not be splittable
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Editor (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla8
Assigned To: Fabien Cazenave [:kaze]
:
Mentors:
Depends on: 680279
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-28 03:45 PDT by Fabien Cazenave [:kaze]
Modified: 2011-08-18 15:39 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch proposal (1.84 KB, patch)
2011-08-01 06:59 PDT, Fabien Cazenave [:kaze]
no flags Details | Diff | Splinter Review
testcase (958 bytes, text/html)
2011-08-01 07:41 PDT, Fabien Cazenave [:kaze]
no flags Details
patch proposal (7.37 KB, patch)
2011-08-01 10:17 PDT, Fabien Cazenave [:kaze]
ehsan: review+
Details | Diff | Splinter Review
patch proposal (7.37 KB, patch)
2011-08-02 08:59 PDT, Fabien Cazenave [:kaze]
ehsan: review+
Details | Diff | Splinter Review

Description Fabien Cazenave [:kaze] 2011-07-28 03:45:54 PDT
In design mode, pressing [Return] twice in a list (ol, ul, dl) splits the list and inserts a paragraph. When the list is the active editing host, it should not be split.

Reproducible: Always

Steps to Reproduce:
1. create an empty HTML file with the following code:
    <ul contenteditable>
        <li> foo </li>
        <li> bar </li>
    </ul>
2. put the caret at the end of the first list item
3. press [Return] twice
4. the resulting code is:
    <ul contenteditable>
        <li> foo </li>
    </ul>
    <p> <br _moz_dirty> </p>
    <ul contenteditable>
        <li> bar </li>
    </ul>

Expected Results:
    <ul contenteditable>
        <li> foo </li>
        <li> <br _moz_dirty> </li>
        <li> <br _moz_dirty> </li>
        <li> bar </li>
    </ul>
Comment 1 Fabien Cazenave [:kaze] 2011-07-28 03:46:58 PDT
bug 597349 and bug 570144 might be related.
Comment 2 Fabien Cazenave [:kaze] 2011-08-01 06:59:04 PDT
Created attachment 549776 [details] [diff] [review]
patch proposal
Comment 3 Fabien Cazenave [:kaze] 2011-08-01 07:41:14 PDT
Created attachment 549791 [details]
testcase
Comment 4 Fabien Cazenave [:kaze] 2011-08-01 10:17:05 PDT
Created attachment 549839 [details] [diff] [review]
patch proposal

slightly factorized code + unit tests
Comment 5 :Ehsan Akhgari 2011-08-02 07:57:20 PDT
Comment on attachment 549839 [details] [diff] [review]
patch proposal

Review of attachment 549839 [details] [diff] [review]:
-----------------------------------------------------------------

::: editor/libeditor/html/nsHTMLEditRules.cpp
@@ +6782,2 @@
>    // if we are in an empty listitem, then we want to pop up out of the list
> +  // but only if prefs says it's ok and if the parent isn't the active editing host.

Nit: if the list container isn't the active editing host.
Comment 6 Fabien Cazenave [:kaze] 2011-08-02 08:59:18 PDT
Created attachment 550096 [details] [diff] [review]
patch proposal

same patch w/ fixed comment.
Comment 7 :Ehsan Akhgari 2011-08-02 11:55:47 PDT
Comment on attachment 550096 [details] [diff] [review]
patch proposal

(You didn't really need a review for this one :-)
Comment 8 :Ehsan Akhgari 2011-08-02 12:18:24 PDT
Pushed to inbound.
Comment 9 Marco Bonardo [::mak] 2011-08-03 02:18:22 PDT
http://hg.mozilla.org/mozilla-central/rev/6ea906365d1f

Note You need to log in before you can comment on or make changes to this bug.