Changeset 83247 in webkit

Timestamp:

Apr 7, 2011 9:11:13 PM (13 years ago)

Author:

rniwa@webkit.org

Message:

2011-04-07 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Darin Adler.

REGRESSION (r46914, r48764): When typing in Mail, line wrapping frequently occurs in the middle of words
​https://bugs.webkit.org/show_bug.cgi?id=57872

Added tests to ensure WebKit inserts a paragraph separator properly around tab span.

• editing/inserting/insert-div-021-expected.txt: No longer duplicates span[id="test"] incorrectly.
• editing/inserting/insert-paragraph-after-tab-span-and-text-expected.txt: Added.
• editing/inserting/insert-paragraph-after-tab-span-and-text.html: Added.
• editing/inserting/insert-paragraph-separator-tab-span-expected.txt: Added.
• editing/inserting/insert-paragraph-separator-tab-span.html: Added.
• editing/inserting/insert-paragraph-at-end-of-line-expected.txt: No longer duplicates a[id="anchor"] incorrectly.

2011-04-07 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Eric Seidel.

REGRESSION (r46914, r48764): When typing in Mail, line wrapping frequently occurs in the middle of words
​https://bugs.webkit.org/show_bug.cgi?id=57872

r46914 initially introduced a regression by replacing calls to styleAtPosition by editingStyleAtPosition
because editingStyleAtPosition did not avoid tab span to obtain the computed style unlike styleAtPosition.

r46914 also introduced a regression by cloning hierarchy under new block at the insertion position without
avoiding the tab span.

Fixed the both regressions by avoiding tab spans when computing the editing style and when cloning hierarchy.
Also reverted r46914 for the general code path because re-creating node hierarchy duplicates nodes when
we're moving nodes after the paragraph separator. Instead, we now split the tree up until the start block
before moving the nodes.

Tests: editing/inserting/insert-paragraph-after-tab-span-and-text.html

editing/inserting/insert-paragraph-separator-tab-span.html

• editing/DeleteSelectionCommand.cpp: (WebCore::DeleteSelectionCommand::saveTypingStyleState): Since EditingStyle's constructor avoids a tab span, no longer calls positionBeforeTabSpan on the position passed to EditingStyle's constructor.
• editing/EditingStyle.cpp: (WebCore::EditingStyle::init): Always avoid a tab span when computing the editing style.
• editing/InsertParagraphSeparatorCommand.cpp: (WebCore::InsertParagraphSeparatorCommand::doApply): Avoid cloning tab spans and inserting a paragraph separator into a paragraph separator.
• editing/htmlediting.cpp: (WebCore::positionOutsideTabSpan): Renamed from positionBeforeTabSpan. Also returns the position in the parent node after the tab span if the position was at the end of the tab span.
• editing/htmlediting.h:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (2 diffs)
• LayoutTests/editing/inserting/insert-div-021-expected.txt (modified) (1 diff)
• LayoutTests/editing/inserting/insert-paragraph-after-tab-span-and-text-expected.txt (added)
• LayoutTests/editing/inserting/insert-paragraph-after-tab-span-and-text.html (added)
• LayoutTests/editing/inserting/insert-paragraph-at-end-of-line-expected.txt (modified) (1 diff)
• LayoutTests/editing/inserting/insert-paragraph-separator-tab-span-expected.txt (added)
• LayoutTests/editing/inserting/insert-paragraph-separator-tab-span.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/editing/DeleteSelectionCommand.cpp (modified) (1 diff)
• Source/WebCore/editing/EditingStyle.cpp (modified) (1 diff)
• Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp (modified) (6 diffs)
• Source/WebCore/editing/htmlediting.cpp (modified) (1 diff)
• Source/WebCore/editing/htmlediting.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-04-07  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        REGRESSION (r46914, r48764): When typing in Mail, line wrapping frequently occurs in the middle of words
+        https://bugs.webkit.org/show_bug.cgi?id=57872
+
+        Added tests to ensure WebKit inserts a paragraph separator properly around tab span.
+
+        * editing/inserting/insert-div-021-expected.txt: No longer duplicates span[id="test"] incorrectly.
+        * editing/inserting/insert-paragraph-after-tab-span-and-text-expected.txt: Added.
+        * editing/inserting/insert-paragraph-after-tab-span-and-text.html: Added.
+        * editing/inserting/insert-paragraph-separator-tab-span-expected.txt: Added.
+        * editing/inserting/insert-paragraph-separator-tab-span.html: Added.
+        * editing/inserting/insert-paragraph-at-end-of-line-expected.txt: No longer duplicates a[id="anchor"] incorrectly.
+
 2011-04-07  Ryosuke Niwa  <rniwa@webkit.org>
 
@@ -408 +424 @@
         * platform/chromium-mac-leopard/fast/speech/input-appearance-searchandspeech-expected.png: Added.
         * platform/chromium-mac/fast/speech/input-appearance-numberandspeech-expected.txt:
+
+2011-04-07  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        REGRESSION (r46914, r48764): When typing in Mail, line wrapping frequently occurs in the middle of words
+        https://bugs.webkit.org/show_bug.cgi?id=57872
+
+        Added a test insert a paragraph separator and text around tab spans. WebKit should not apply the tab span's
+        style to the paragraph separator or the text.
+
+        * editing/inserting/insert-paragraph-separator-tab-span-expected.txt: Added.
+        * editing/inserting/insert-paragraph-separator-tab-span.html: Added.
 
 2011-04-07  Ryosuke Niwa  <rniwa@webkit.org>

trunk/LayoutTests/editing/inserting/insert-div-021-expected.txt

@@ -33 +33 @@
 |   <br>
 | <div>
-|   <span>
-|     id="test"
 |   "<#selection-caret>bar
 "

trunk/LayoutTests/editing/inserting/insert-paragraph-at-end-of-line-expected.txt

@@ -19 +19 @@
 |   "1"
 | <div>
-|   <a>
-|     href="#"
-|     id="anchor"
 |   "2<#selection-caret>"
 |   <br>

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-04-07  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        REGRESSION (r46914, r48764): When typing in Mail, line wrapping frequently occurs in the middle of words
+        https://bugs.webkit.org/show_bug.cgi?id=57872
+
+        r46914 initially introduced a regression by replacing calls to styleAtPosition by editingStyleAtPosition
+        because editingStyleAtPosition did not avoid tab span to obtain the computed style unlike styleAtPosition.
+
+        r46914 also introduced a regression by cloning hierarchy under new block at the insertion position without
+        avoiding the tab span.
+
+        Fixed the both regressions by avoiding tab spans when computing the editing style and when cloning hierarchy.
+        Also reverted r46914 for the general code path because re-creating node hierarchy duplicates nodes when
+        we're moving nodes after the paragraph separator. Instead, we now split the tree up until the start block
+        before moving the nodes.
+
+        Tests: editing/inserting/insert-paragraph-after-tab-span-and-text.html
+               editing/inserting/insert-paragraph-separator-tab-span.html
+
+        * editing/DeleteSelectionCommand.cpp:
+        (WebCore::DeleteSelectionCommand::saveTypingStyleState): Since EditingStyle's constructor avoids a tab span,
+        no longer calls positionBeforeTabSpan on the position passed to EditingStyle's constructor.
+        * editing/EditingStyle.cpp:
+        (WebCore::EditingStyle::init): Always avoid a tab span when computing the editing style.
+        * editing/InsertParagraphSeparatorCommand.cpp:
+        (WebCore::InsertParagraphSeparatorCommand::doApply): Avoid cloning tab spans and inserting a paragraph
+        separator into a paragraph separator.
+        * editing/htmlediting.cpp:
+        (WebCore::positionOutsideTabSpan): Renamed from positionBeforeTabSpan. Also returns the position in the parent
+        node after the tab span if the position was at the end of the tab span.
+        * editing/htmlediting.h:
+
 2011-04-07  Jia Pu  <jpu@apple.com>
 

trunk/Source/WebCore/editing/DeleteSelectionCommand.cpp

@@ -282 +282 @@
 
     // Figure out the typing style in effect before the delete is done.
-    m_typingStyle = EditingStyle::create(positionBeforeTabSpan(m_selectionToDelete.start()));
+    m_typingStyle = EditingStyle::create(m_selectionToDelete.start());
     m_typingStyle->removeStyleAddedByNode(enclosingAnchorElement(m_selectionToDelete.start()));
 

trunk/Source/WebCore/editing/EditingStyle.cpp

@@ -301 +301 @@
 void EditingStyle::init(Node* node, PropertiesToInclude propertiesToInclude)
 {
+    if (isTabSpanTextNode(node))
+        node = tabSpanNode(node)->parentNode();
+    else if (isTabSpanNode(node))
+        node = node->parentNode();
+
     RefPtr<CSSComputedStyleDeclaration> computedStyleAtPosition = computedStyle(node);
     m_mutableStyle = propertiesToInclude == AllProperties && computedStyleAtPosition ? computedStyleAtPosition->copy() : editingStyleFromComputedStyle(computedStyleAtPosition);

trunk/Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp

@@ -240 +240 @@
         
         Vector<Element*> ancestors;
-        getAncestorsInsideBlock(insertionPosition.deprecatedNode(), startBlock, ancestors);      
+        getAncestorsInsideBlock(positionOutsideTabSpan(insertionPosition).deprecatedNode(), startBlock, ancestors);      
         RefPtr<Element> parent = cloneHierarchyUnderNewBlock(ancestors, blockToInsert);
         
@@ -255 +255 @@
     if (isFirstInBlock || !inSameBlock(visiblePos, visiblePos.previous())) {
         Node *refNode;
+        
+        insertionPosition = positionOutsideTabSpan(insertionPosition);
+
         if (isFirstInBlock && !nestNewBlock)
             refNode = startBlock;
@@ -271 +274 @@
 
         Vector<Element*> ancestors;
-        getAncestorsInsideBlock(positionAvoidingSpecialElementBoundary(insertionPosition).deprecatedNode(), startBlock, ancestors);
+        getAncestorsInsideBlock(positionAvoidingSpecialElementBoundary(positionOutsideTabSpan(insertionPosition)).deprecatedNode(), startBlock, ancestors);
         
         appendBlockPlaceholder(cloneHierarchyUnderNewBlock(ancestors, blockToInsert));
@@ -300 +303 @@
     // all of the correct nodes when building the ancestor list.  So this needs to be the deepest representation of the position
     // before we walk the DOM tree.
-    insertionPosition = VisiblePosition(insertionPosition).deepEquivalent();
-
-    // Build up list of ancestors in between the start node and the start block.
-    Vector<Element*> ancestors;
-    getAncestorsInsideBlock(insertionPosition.deprecatedNode(), startBlock, ancestors);
+    insertionPosition = positionOutsideTabSpan(VisiblePosition(insertionPosition).deepEquivalent());
 
     // Make sure we do not cause a rendered space to become unrendered.
@@ -336 +335 @@
 
     updateLayout();
-    
-    // Make clones of ancestors in between the start node and the outer block.
-    RefPtr<Element> parent = cloneHierarchyUnderNewBlock(ancestors, blockToInsert);
 
     // If the paragraph separator was inserted at the end of a paragraph, an empty line must be
@@ -345 +341 @@
     if (isEndOfParagraph(visiblePos) && !lineBreakExistsAtVisiblePosition(visiblePos))
         appendNode(createBreakElement(document()).get(), blockToInsert.get());
-        
+
     // Move the start node and the siblings of the start node.
-    if (insertionPosition.deprecatedNode() != startBlock) {
-        Node* n = insertionPosition.deprecatedNode();
-        if (insertionPosition.deprecatedEditingOffset() >= caretMaxOffset(n))
-            n = n->nextSibling();
+    if (VisiblePosition(insertionPosition) != VisiblePosition(positionBeforeNode(blockToInsert.get()))) {
+        Node* n;
+        if (insertionPosition.containerNode() == startBlock)
+            n = insertionPosition.computeNodeAfterPosition();
+        else {
+            splitTreeToNode(insertionPosition.containerNode(), startBlock);
+
+            for (n = startBlock->firstChild(); n; n = n->nextSibling()) {
+                if (comparePositions(VisiblePosition(insertionPosition), positionBeforeNode(n)) <= 0)
+                    break;
+            }
+        }
 
         while (n && n != blockToInsert) {
             Node *next = n->nextSibling();
             removeNode(n);
-            appendNode(n, parent.get());
+            appendNode(n, blockToInsert);
             n = next;
         }
     }            
-
-    // Move everything after the start node.
-    if (!ancestors.isEmpty()) {
-        Element* leftParent = ancestors.first();
-        while (leftParent && leftParent != startBlock) {
-            parent = parent->parentElement();
-            if (!parent)
-                break;
-            Node* n = leftParent->nextSibling();
-            while (n && n != blockToInsert) {
-                Node* next = n->nextSibling();
-                removeNode(n);
-                appendNode(n, parent.get());
-                n = next;
-            }
-            leftParent = leftParent->parentElement();
-        }
-    }
 
     // Handle whitespace that occurs after the split

trunk/Source/WebCore/editing/htmlediting.cpp

@@ -895 +895 @@
 }
     
-Position positionBeforeTabSpan(const Position& pos)
-{
-    Node* node = pos.deprecatedNode();
+Position positionOutsideTabSpan(const Position& pos)
+{
+    Node* node = pos.containerNode();
     if (isTabSpanTextNode(node))
         node = tabSpanNode(node);
     else if (!isTabSpanNode(node))
         return pos;
-    
+
+    if (node && VisiblePosition(pos) == lastPositionInNode(node))
+        return positionInParentAfterNode(node);
+
     return positionInParentBeforeNode(node);
 }

trunk/Source/WebCore/editing/htmlediting.h

@@ -112 +112 @@
 Position previousVisuallyDistinctCandidate(const Position&);
 
-Position positionBeforeTabSpan(const Position&);
+Position positionOutsideTabSpan(const Position&);
 Position positionBeforeContainingSpecialElement(const Position&, Node** containingSpecialElement=0);
 Position positionAfterContainingSpecialElement(const Position&, Node** containingSpecialElement=0);