Title: [119270] trunk
Revision
119270
Author
commit-qu...@webkit.org
Date
2012-06-01 13:08:19 -0700 (Fri, 01 Jun 2012)

Log Message

Indenting a paragraph that begins with a link 3 times breaks the paragraph into two paragraphs
https://bugs.webkit.org/show_bug.cgi?id=87428

Patch by Shezan Baig <shezbaig...@gmail.com> on 2012-06-01
Reviewed by Ryosuke Niwa.

Source/WebCore:

Fix the way lastNode (our insertion point) is updated whenever
traverseNextSibling moves up to a new parent, so that the relative
depth between the next sibling and the original start node is
maintained in the clone. The divergence in depth broke the paragraph
into two paragraphs because the next sibling was inserted outside the
blockquote that was created for the indentation.

Note that the topNode is not required anymore because it is no longer
used anywhere.

Tests: editing/execCommand/indent-nested-inlines-1.html
       editing/execCommand/indent-nested-inlines-2.html

* editing/CompositeEditCommand.cpp:
(WebCore::CompositeEditCommand::cloneParagraphUnderNewElement):

LayoutTests:

Adding two tests for indenting nested inlines.

* editing/execCommand/indent-nested-inlines-1-expected.txt: Added.
* editing/execCommand/indent-nested-inlines-1.html: Added.
* editing/execCommand/indent-nested-inlines-2-expected.txt: Added.
* editing/execCommand/indent-nested-inlines-2.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (119269 => 119270)


--- trunk/LayoutTests/ChangeLog	2012-06-01 20:00:43 UTC (rev 119269)
+++ trunk/LayoutTests/ChangeLog	2012-06-01 20:08:19 UTC (rev 119270)
@@ -1,3 +1,17 @@
+2012-06-01  Shezan Baig  <shezbaig...@gmail.com>
+
+        Indenting a paragraph that begins with a link 3 times breaks the paragraph into two paragraphs
+        https://bugs.webkit.org/show_bug.cgi?id=87428
+
+        Reviewed by Ryosuke Niwa.
+
+        Adding two tests for indenting nested inlines.
+
+        * editing/execCommand/indent-nested-inlines-1-expected.txt: Added.
+        * editing/execCommand/indent-nested-inlines-1.html: Added.
+        * editing/execCommand/indent-nested-inlines-2-expected.txt: Added.
+        * editing/execCommand/indent-nested-inlines-2.html: Added.
+
 2012-06-01  Ami Fischman  <fisch...@chromium.org>
 
         [chromium] Unskip http/tests/media/video-buffered.html

Added: trunk/LayoutTests/editing/execCommand/indent-nested-inlines-1-expected.txt (0 => 119270)


--- trunk/LayoutTests/editing/execCommand/indent-nested-inlines-1-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/execCommand/indent-nested-inlines-1-expected.txt	2012-06-01 20:08:19 UTC (rev 119270)
@@ -0,0 +1,11 @@
+This tests indenting paragraphs that begin with inlines.
+| <blockquote>
+|   style="margin: 0 0 0 40px; border: none; padding: 0px;"
+|   <blockquote>
+|     style="margin: 0 0 0 40px; border: none; padding: 0px;"
+|     <blockquote>
+|       style="margin: 0 0 0 40px; border: none; padding: 0px;"
+|       <a>
+|         href=""
+|         "<#selection-anchor>Link"
+|       "This should be under same the blockquote as Link<#selection-focus>"

Added: trunk/LayoutTests/editing/execCommand/indent-nested-inlines-1.html (0 => 119270)


--- trunk/LayoutTests/editing/execCommand/indent-nested-inlines-1.html	                        (rev 0)
+++ trunk/LayoutTests/editing/execCommand/indent-nested-inlines-1.html	2012-06-01 20:08:19 UTC (rev 119270)
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<div contenteditable="true"><a href="" should be under same the blockquote as Link</div>
+<script>
+
+Markup.description('This tests indenting paragraphs that begin with inlines.');
+
+var div = document.getElementsByTagName('div')[0];
+window.getSelection().selectAllChildren(div);
+document.execCommand('indent', false, true);
+document.execCommand('indent', false, true);
+document.execCommand('indent', false, true);
+
+Markup.dump(div);
+
+</script>
+</body>
+</html>
Property changes on: trunk/LayoutTests/editing/execCommand/indent-nested-inlines-1.html
___________________________________________________________________

Added: svn:executable

Added: trunk/LayoutTests/editing/execCommand/indent-nested-inlines-2-expected.txt (0 => 119270)


--- trunk/LayoutTests/editing/execCommand/indent-nested-inlines-2-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/execCommand/indent-nested-inlines-2-expected.txt	2012-06-01 20:08:19 UTC (rev 119270)
@@ -0,0 +1,12 @@
+This tests indenting paragraphs that begin with nested inlines.
+| <blockquote>
+|   style="margin: 0 0 0 40px; border: none; padding: 0px;"
+|   <blockquote>
+|     style="margin: 0 0 0 40px; border: none; padding: 0px;"
+|     <blockquote>
+|       style="margin: 0 0 0 40px; border: none; padding: 0px;"
+|       <b>
+|         <a>
+|           href=""
+|           "<#selection-anchor>BoldLink"
+|       "This should be under the same blockquote as BoldLink, and not bold<#selection-focus>"

Added: trunk/LayoutTests/editing/execCommand/indent-nested-inlines-2.html (0 => 119270)


--- trunk/LayoutTests/editing/execCommand/indent-nested-inlines-2.html	                        (rev 0)
+++ trunk/LayoutTests/editing/execCommand/indent-nested-inlines-2.html	2012-06-01 20:08:19 UTC (rev 119270)
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<div contenteditable="true"><b><a href="" should be under the same blockquote as BoldLink, and not bold</div>
+<script>
+
+Markup.description('This tests indenting paragraphs that begin with nested inlines.');
+
+var div = document.getElementsByTagName('div')[0];
+window.getSelection().selectAllChildren(div);
+document.execCommand('indent', false, true);
+document.execCommand('indent', false, true);
+document.execCommand('indent', false, true);
+
+Markup.dump(div);
+
+</script>
+</body>
+</html>
Property changes on: trunk/LayoutTests/editing/execCommand/indent-nested-inlines-2.html
___________________________________________________________________

Added: svn:executable

Modified: trunk/Source/WebCore/ChangeLog (119269 => 119270)


--- trunk/Source/WebCore/ChangeLog	2012-06-01 20:00:43 UTC (rev 119269)
+++ trunk/Source/WebCore/ChangeLog	2012-06-01 20:08:19 UTC (rev 119270)
@@ -1,3 +1,26 @@
+2012-06-01  Shezan Baig  <shezbaig...@gmail.com>
+
+        Indenting a paragraph that begins with a link 3 times breaks the paragraph into two paragraphs
+        https://bugs.webkit.org/show_bug.cgi?id=87428
+
+        Reviewed by Ryosuke Niwa.
+
+        Fix the way lastNode (our insertion point) is updated whenever
+        traverseNextSibling moves up to a new parent, so that the relative
+        depth between the next sibling and the original start node is
+        maintained in the clone. The divergence in depth broke the paragraph
+        into two paragraphs because the next sibling was inserted outside the
+        blockquote that was created for the indentation.
+
+        Note that the topNode is not required anymore because it is no longer
+        used anywhere.
+
+        Tests: editing/execCommand/indent-nested-inlines-1.html
+               editing/execCommand/indent-nested-inlines-2.html
+
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::cloneParagraphUnderNewElement):
+
 2012-06-01  Joe Thomas  <joetho...@motorola.com>
 
         getComputedStyle for background shorthand property does not return background-origin and background-clip.

Modified: trunk/Source/WebCore/editing/CompositeEditCommand.cpp (119269 => 119270)


--- trunk/Source/WebCore/editing/CompositeEditCommand.cpp	2012-06-01 20:00:43 UTC (rev 119269)
+++ trunk/Source/WebCore/editing/CompositeEditCommand.cpp	2012-06-01 20:08:19 UTC (rev 119270)
@@ -967,17 +967,14 @@
 void CompositeEditCommand::cloneParagraphUnderNewElement(Position& start, Position& end, Node* passedOuterNode, Element* blockElement)
 {
     // First we clone the outerNode
-    RefPtr<Node> topNode;
     RefPtr<Node> lastNode;
     RefPtr<Node> outerNode = passedOuterNode;
 
     if (outerNode->isRootEditableElement()) {
-        topNode = blockElement;
         lastNode = blockElement;
     } else {
-        topNode = outerNode->cloneNode(isTableElement(outerNode.get()));
-        appendNode(topNode, blockElement);
-        lastNode = topNode;
+        lastNode = outerNode->cloneNode(isTableElement(outerNode.get()));
+        appendNode(lastNode, blockElement);
     }
 
     if (start.deprecatedNode() != outerNode && lastNode->isElementNode()) {
@@ -1002,21 +999,26 @@
     
     if (start.deprecatedNode() != end.deprecatedNode() && !start.deprecatedNode()->isDescendantOf(end.deprecatedNode())) {
         // If end is not a descendant of outerNode we need to
-        // find the first common ancestor and adjust the insertion
-        // point accordingly.
+        // find the first common ancestor to increase the scope
+        // of our nextSibling traversal.
         while (!end.deprecatedNode()->isDescendantOf(outerNode.get())) {
             outerNode = outerNode->parentNode();
-            topNode = topNode->parentNode();
         }
 
-        for (Node* n = start.deprecatedNode()->traverseNextSibling(outerNode.get()); n; n = n->traverseNextSibling(outerNode.get())) {
-            if (n->parentNode() != start.deprecatedNode()->parentNode())
-                lastNode = topNode->lastChild();
+        Node* startNode = start.deprecatedNode();
+        for (Node* node = startNode->traverseNextSibling(outerNode.get()); node; node = node->traverseNextSibling(outerNode.get())) {
+            // Move lastNode up in the tree as much as node was moved up in the
+            // tree by traverseNextSibling, so that the relative depth between
+            // node and the original start node is maintained in the clone.
+            while (startNode->parentNode() != node->parentNode()) {
+                startNode = startNode->parentNode();
+                lastNode = lastNode->parentNode();
+            }
 
-            RefPtr<Node> clonedNode = n->cloneNode(true);
+            RefPtr<Node> clonedNode = node->cloneNode(true);
             insertNodeAfter(clonedNode, lastNode);
             lastNode = clonedNode.release();
-            if (n == end.deprecatedNode() || end.deprecatedNode()->isDescendantOf(n))
+            if (node == end.deprecatedNode() || end.deprecatedNode()->isDescendantOf(node))
                 break;
         }
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to