Attachment 552911 Details for Bug 676401

[patch] patch proposal

bug674861.patch (text/plain), 7.37 KB, created by Fabien Cazenave [:kaze]

(hide)

Description:

Filename:

MIME Type:

Creator: Fabien Cazenave [:kaze]

Size: 7.37 KB

patch

obsolete

># HG changeset patch
># Parent f5f1e38225404126875f536c2875ca6a70150bca
># User Fabien Cazenave <kaze@kompozer.net>
>Bug 674861 - contentEditable lists should not be splittable
>
>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.
>
>diff --git a/editor/libeditor/html/nsHTMLEditRules.cpp b/editor/libeditor/html/nsHTMLEditRules.cpp
>--- a/editor/libeditor/html/nsHTMLEditRules.cpp
>+++ b/editor/libeditor/html/nsHTMLEditRules.cpp
>@@ -6766,41 +6766,48 @@ nsHTMLEditRules::ReturnInListItem(nsISel
>   nsresult res = NS_OK;
>   
>   nsCOMPtr<nsIDOMNode> listitem;
>   
>   // sanity check
>   NS_PRECONDITION(PR_TRUE == nsHTMLEditUtils::IsListItem(aListItem),
>                   "expected a list item and didn't get one");
>   
>+  // get the listitem parent and the active editing host.
>+  nsIContent* rootContent = mHTMLEditor->GetActiveEditingHost();
>+  nsCOMPtr<nsIDOMNode> rootNode = do_QueryInterface(rootContent);
>+  nsCOMPtr<nsIDOMNode> list;
>+  PRInt32 itemOffset;
>+  res = nsEditor::GetNodeLocation(aListItem, address_of(list), &itemOffset);
>+  NS_ENSURE_SUCCESS(res, res);
>+
>   // 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 && mReturnInEmptyLIKillsList)   // but only if prefs says it's ok
>-  {
>-    nsCOMPtr<nsIDOMNode> list, listparent;
>-    PRInt32 offset, itemOffset;
>-    res = nsEditor::GetNodeLocation(aListItem, address_of(list), &itemOffset);
>-    NS_ENSURE_SUCCESS(res, res);
>-    res = nsEditor::GetNodeLocation(list, address_of(listparent), &offset);
>-    NS_ENSURE_SUCCESS(res, res);
>-    
>+  if (isEmpty && (rootNode != list) && mReturnInEmptyLIKillsList)
>+  {
>     // 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
>diff --git a/editor/libeditor/html/tests/Makefile.in b/editor/libeditor/html/tests/Makefile.in
>--- a/editor/libeditor/html/tests/Makefile.in
>+++ b/editor/libeditor/html/tests/Makefile.in
>@@ -50,17 +50,17 @@ _TEST_FILES = \
> 		test_bug332636.html^headers^ \
> 		test_bug366682.html \
> 		test_bug372345.html \
> 		test_bug410986.html \
> 		test_bug414526.html \
> 		test_bug417418.html \
> 		test_bug432225.html \
> 		test_bug439808.html \
>- 		test_bug449243.html \
>+		test_bug449243.html \
> 		test_bug455992.html \
> 		test_bug456244.html \
> 		test_bug460740.html \
> 		test_bug478725.html \
> 		test_bug480972.html \
> 		test_bug484181.html \
> 		test_bug487524.html \
> 		test_bug520189.html \
>@@ -76,16 +76,17 @@ _TEST_FILES = \
> 		test_bug607584.html \
> 		test_bug611182.html \
> 		test_bug612447.html \
> 		test_bug620906.html \
> 		test_bug622371.html \
> 		test_bug629845.html \
> 		test_bug640321.html \
> 		test_bug668599.html \
>+		test_bug674861.html \
> 		test_CF_HTML_clipboard.html \
> 		test_contenteditable_focus.html \
> 		test_htmleditor_keyevent_handling.html \
> 		test_select_all_without_body.html \
> 		file_select_all_without_body.html \
> 		test_root_element_replacement.html \
> 		$(NULL)
> 
>diff --git a/editor/libeditor/html/tests/test_bug674861.html b/editor/libeditor/html/tests/test_bug674861.html
>new file mode 100644
>--- /dev/null
>+++ b/editor/libeditor/html/tests/test_bug674861.html
>@@ -0,0 +1,120 @@
>+<!DOCTYPE HTML>
>+<html>
>+
>+<head>
>+  <title>Test for Bug 674861</title>
>+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
>+  <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>
>+
>+  <h2> Ordered List </h2>
>+  <ol contenteditable>
>+    <li> item 1 </li>
>+    <li> item 2 </li>
>+    <li> item 3 </li>
>+  </ol>
>+
>+  <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>
>+</div>
>+
>+<pre id="test">
>+<script type="application/javascript">
>+
>+/** Test for Bug 674861 **/
>+SimpleTest.waitForExplicitFinish();
>+SimpleTest.waitForFocus(runTests);
>+
>+const CARET_BEGIN  = 0;
>+const CARET_MIDDLE = 1;
>+const CARET_END    = 2;
>+
>+function try2split(element, caretPos) {
>+  // compute the requested position
>+  var len = element.textContent.length;
>+  var pos = -1;
>+  switch (caretPos) {
>+    case CARET_BEGIN:
>+      pos = 0;
>+      break;
>+    case CARET_MIDDLE:
>+      pos = Math.floor(len/2);
>+      break;
>+    case CARET_END:
>+      pos = len;
>+      break;
>+  }
>+
>+  // put the caret on the requested position
>+  var sel = window.getSelection();
>+  for (var i = 0; i < sel.rangeCount; i++) {
>+    var range = sel.getRangeAt(i);
>+    sel.removeRange(range);
>+  }
>+  range = document.createRange();
>+  range.setStart(element.firstChild, pos);
>+  range.setEnd(element.firstChild, pos);
>+  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");
>+
>+  // 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(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(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(dl.querySelectorAll("dt").length, 5,
>+    "Two new <dt> elements should have been created.");
>+
>+  // done
>+  SimpleTest.finish();
>+}
>+</script>
>+</pre>
>+</body>
>+</html>

Actions: View | Diff | Review

Attachments on bug 676401: 552870 | 552877 | 552911 | 553191 | 553429