Bug 680279 - Splitting Gmail rich-text list with Enter misplaces the caret; r=ehsan
authorFabien Cazenave <kaze@kompozer.net>
Fri, 19 Aug 2011 18:14:04 -0400
changeset 75587 d6745c14f05147c46148571f3590b40929521efe
parent 75586 139eed687d7736857901d759b8cc3038e91d1685
child 75588 ecef6506d094b2cbdb504d68becfbb6d6ecb86ce
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewersehsan
bugs680279, 674861
milestone9.0a1
Bug 680279 - Splitting Gmail rich-text list with Enter misplaces the caret; r=ehsan The patch for bug 674861 has introduced a regression. When a list is splitted, a new paragraph should inserted before the new list.
editor/libeditor/html/nsHTMLEditRules.cpp
editor/libeditor/html/tests/test_bug674861.html
--- a/editor/libeditor/html/nsHTMLEditRules.cpp
+++ b/editor/libeditor/html/nsHTMLEditRules.cpp
@@ -6784,33 +6784,35 @@ nsHTMLEditRules::ReturnInListItem(nsISel
 
   // 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.
   PRBool isEmpty;
   res = IsEmptyBlock(aListItem, &isEmpty, PR_TRUE, PR_FALSE);
   NS_ENSURE_SUCCESS(res, res);
   if (isEmpty && (rootNode != list) && mReturnInEmptyLIKillsList)
   {
+    // get the list offset now -- before we might eventually split the list
+    nsCOMPtr<nsIDOMNode> listparent;
+    PRInt32 offset;
+    res = nsEditor::GetNodeLocation(list, address_of(listparent), &offset);
+    NS_ENSURE_SUCCESS(res, res);
+
     // are we the last list item in the list?
     PRBool bIsLast;
     res = mHTMLEditor->IsLastEditableChild(aListItem, &bIsLast);
     NS_ENSURE_SUCCESS(res, res);
     if (!bIsLast)
     {
       // we need to split the list!
       nsCOMPtr<nsIDOMNode> tempNode;
       res = mHTMLEditor->SplitNode(list, itemOffset, getter_AddRefs(tempNode));
       NS_ENSURE_SUCCESS(res, res);
     }
 
     // are we in a sublist?
-    nsCOMPtr<nsIDOMNode> listparent;
-    PRInt32 offset;
-    res = nsEditor::GetNodeLocation(list, address_of(listparent), &offset);
-    NS_ENSURE_SUCCESS(res, res);
     if (nsHTMLEditUtils::IsList(listparent))  //in a sublist
     {
       // if so, move this list item out of this list and into the grandparent list
       res = mHTMLEditor->MoveNode(aListItem,listparent,offset+1);
       NS_ENSURE_SUCCESS(res, res);
       res = aSelection->Collapse(aListItem,0);
     }
     else
--- a/editor/libeditor/html/tests/test_bug674861.html
+++ b/editor/libeditor/html/tests/test_bug674861.html
@@ -8,39 +8,67 @@ https://bugzilla.mozilla.org/show_bug.cg
   <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
   <script type="text/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=674861">Mozilla Bug 674861</a>
 <p id="display"></p>
 <div id="content">
-  <h2> Bullet List </h2>
-  <ul contenteditable>
-    <li> item 1 </li>
-    <li> item 2 </li>
-    <li> item 3 </li>
-  </ul>
+  <section id="test1">
+    <h2> Editable Bullet List </h2>
+    <ul contenteditable>
+      <li> item A </li>
+      <li> item B </li>
+      <li> item C </li>
+    </ul>
+
+    <h2> Editable Ordered List </h2>
+    <ol contenteditable>
+      <li> item A </li>
+      <li> item B </li>
+      <li> item C </li>
+    </ol>
+
+    <h2> Editable Definition List </h2>
+    <dl contenteditable>
+      <dt> term A </dt>
+      <dd> definition A </dd>
+      <dt> term B </dt>
+      <dd> definition B </dd>
+      <dt> term C </dt>
+      <dd> definition C </dd>
+    </dl>
+  </section>
 
-  <h2> Ordered List </h2>
-  <ol contenteditable>
-    <li> item 1 </li>
-    <li> item 2 </li>
-    <li> item 3 </li>
-  </ol>
+  <section id="test2" contenteditable>
+    <h2> Bullet List In Editable Section </h2>
+    <ul>
+      <li> item A </li>
+      <li> item B </li>
+      <li> item C </li>
+    </ul>
 
-  <h2> Definition List </h2>
-  <dl contenteditable>
-    <dt> term 1 </dt>
-    <dd> definition 1 </dd>
-    <dt> term 2 </dt>
-    <dd> definition 2 </dd>
-    <dt> term 3 </dt>
-    <dd> definition 3 </dd>
-  </dl>
+    <h2> Ordered List In Editable Section </h2>
+    <ol>
+      <li> item A </li>
+      <li> item B </li>
+      <li> item C </li>
+    </ol>
+
+    <h2> Definition List In Editable Section </h2>
+    <dl>
+      <dt> term A </dt>
+      <dd> definition A </dd>
+      <dt> term B </dt>
+      <dd> definition B </dd>
+      <dt> term C </dt>
+      <dd> definition C </dd>
+    </dl>
+  </section>
 </div>
 
 <pre id="test">
 <script type="application/javascript">
 
 /** Test for Bug 674861 **/
 SimpleTest.waitForExplicitFinish();
 SimpleTest.waitForFocus(runTests);
@@ -77,43 +105,81 @@ function try2split(element, caretPos) {
   sel.addRange(range);
   
   // simulates two [Return] keypresses
   synthesizeKey("VK_RETURN", {});
   synthesizeKey("VK_RETURN", {});
 }
 
 function runTests() {
-  const ul = document.querySelector("#content ul");
-  const ol = document.querySelector("#content ol");
-  const dl = document.querySelector("#content dl");
+  const test1 = document.getElementById("test1");
+  const test2 = document.getElementById("test2");
+
+  // -----------------------------------------------------------------------
+  // #test1: editable lists should NOT be splittable
+  // -----------------------------------------------------------------------
+  const ul = test1.querySelector("ul");
+  const ol = test1.querySelector("ol");
+  const dl = test1.querySelector("dl");
 
   // bullet list
   ul.focus();
   try2split(ul.querySelector("li"), CARET_END);
-  is(document.querySelectorAll("#content ul").length, 1,
-    "The <ul> list should not be splittable.");
+  is(test1.querySelectorAll("ul").length, 1,
+    "The <ul contenteditable> list should not be splittable.");
   is(ul.querySelectorAll("li").length, 5,
     "Two new <li> elements should have been created.");
 
   // ordered list
   ol.focus();
   try2split(ol.querySelector("li"), CARET_END);
-  is(document.querySelectorAll("#content ol").length, 1,
-    "The <ol> list should not be splittable.");
+  is(test1.querySelectorAll("ol").length, 1,
+    "The <ol contenteditable> list should not be splittable.");
   is(ol.querySelectorAll("li").length, 5,
     "Two new <li> elements should have been created.");
 
   // definition list
   dl.focus();
   try2split(dl.querySelector("dd"), CARET_END);
-  is(document.querySelectorAll("#content dl").length, 1,
-    "The <dl> list should not be splittable.");
+  is(test1.querySelectorAll("dl").length, 1,
+    "The <dl contenteditable> list should not be splittable.");
   is(dl.querySelectorAll("dt").length, 5,
     "Two new <dt> elements should have been created.");
 
+  // -----------------------------------------------------------------------
+  // #test2: lists in editable blocks should be splittable
+  // -----------------------------------------------------------------------
+  test2.focus();
+
+  // bullet list
+  try2split(test2.querySelector("ul li"), CARET_END);
+  is(test2.querySelectorAll("ul").length, 2,
+    "The <ul> list should have been splitted.");
+  is(test2.querySelectorAll("ul li").length, 3,
+    "No new <li> element should have been created.");
+  is(test2.querySelectorAll("ul+p").length, 1,
+    "A new paragraph should have been created.");
+
+  // ordered list
+  try2split(test2.querySelector("ol li"), CARET_END);
+  is(test2.querySelectorAll("ol").length, 2,
+    "The <ol> list should have been splitted.");
+  is(test2.querySelectorAll("ol li").length, 3,
+    "No new <li> element should have been created.");
+  is(test2.querySelectorAll("ol+p").length, 1,
+    "A new paragraph should have been created.");
+
+  // definition list
+  try2split(test2.querySelector("dl dd"), CARET_END);
+  is(test2.querySelectorAll("dl").length, 2,
+    "The <dl> list should have been splitted.");
+  is(test2.querySelectorAll("dt").length, 3,
+    "No new <dt> element should have been created.");
+  is(test2.querySelectorAll("dl+p").length, 1,
+    "A new paragraph should have been created.");
+
   // done
   SimpleTest.finish();
 }
 </script>
 </pre>
 </body>
 </html>