2011-04-06 Ryosuke Niwa <rniwa@webkit.org>
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 6 Apr 2011 12:59:24 +0000 (12:59 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 6 Apr 2011 12:59:24 +0000 (12:59 +0000)
        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.

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

        * 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.
2011-04-06  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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@83039 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/editing/inserting/insert-paragraph-separator-tab-span-expected.txt [new file with mode: 0644]
LayoutTests/editing/inserting/insert-paragraph-separator-tab-span.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/EditingStyle.cpp
Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp

index 64dae57..506cb70 100644 (file)
@@ -1,3 +1,16 @@
+2011-04-06  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-05  Alexander Pavlov  <apavlov@chromium.org>
 
         Reviewed by Pavel Feldman.
diff --git a/LayoutTests/editing/inserting/insert-paragraph-separator-tab-span-expected.txt b/LayoutTests/editing/inserting/insert-paragraph-separator-tab-span-expected.txt
new file mode 100644 (file)
index 0000000..49e1376
--- /dev/null
@@ -0,0 +1,38 @@
+This test ensures WebKit avoids cloning Apple tab span when inserting a paragraph separator.
+Only tab should be inside a Apple tab span in the following tests.
+
+last visible position:
+| <span>
+|   class="Apple-tab-span"
+|   style="white-space:pre"
+|   "  "
+| <div>
+|   "hello world WebKit <#selection-caret>"
+
+first visible position:
+| <div>
+|   <br>
+| "hello world WebKit <#selection-caret>"
+| <span>
+|   class="Apple-tab-span"
+|   style="white-space:pre"
+|   "  "
+
+before tab span:
+| "hi, "
+| <div>
+|   "hello world WebKit <#selection-caret>"
+|   <span>
+|     class="Apple-tab-span"
+|     style="white-space:pre"
+|     "        "
+|   " rocks"
+
+after tab span:
+| "hi, "
+| <span>
+|   class="Apple-tab-span"
+|   style="white-space:pre"
+|   "  "
+| " rockshello world WebKit <#selection-caret>"
+| <div>
diff --git a/LayoutTests/editing/inserting/insert-paragraph-separator-tab-span.html b/LayoutTests/editing/inserting/insert-paragraph-separator-tab-span.html
new file mode 100644 (file)
index 0000000..406ddb7
--- /dev/null
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div contenteditable title="1: last visible position" style="word-wrap: break-word; width: 10ex; border: solid thin black;"><span class="Apple-tab-span" style="white-space:pre">      </span></div>
+<div contenteditable title="0: first visible position" style="word-wrap: break-word; width: 10ex; border: solid thin black;"><span class="Apple-tab-span" style="white-space:pre">     </span></div>
+<div contenteditable title="1: before tab span" style="word-wrap: break-word; width: 10ex; border: solid thin black;">hi, <span class="Apple-tab-span" style="white-space:pre">        </span> rocks</div>
+<div contenteditable title="2: after tab span" style="word-wrap: break-word; width: 10ex; border: solid thin black;">hi, <span class="Apple-tab-span" style="white-space:pre"> </span> rocks</div>
+<script src="../../resources/dump-as-markup.js"></script>
+<script>
+
+Markup.description('This test ensures WebKit avoids cloning Apple tab span when inserting a paragraph separator.\n'+
+    'Only tab should be inside a Apple tab span in the following tests.');
+
+function runTest(div) {
+    var offset = div.title.substr(0, div.title.indexOf(':'));
+    window.getSelection().setPosition(div, offset);
+    document.execCommand("InsertParagraph");
+    document.execCommand("InsertText", false, "hello world WebKit ");
+    Markup.dump(div, div.title.substr(div.title.indexOf(': ') + 2));
+}
+
+var divs = document.getElementsByTagName('div');
+var tests = new Array();
+for (var i = 0; i < divs.length; i++)
+    tests.push(divs[i]);
+
+for (i in tests)
+    runTest(tests[i]);
+
+</script>
+</body>
+</html>
index 0d6c381..93d6946 100644 (file)
@@ -1,3 +1,26 @@
+2011-04-06  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.
+
+        Test: editing/inserting/insert-paragraph-separator-tab-span.html
+
+        * 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.
+
 2011-04-06  Levi Weintraub  <leviw@chromium.org>
 
         Reviewed by Ryosuke Niwa.
index 668c943..b2195a9 100644 (file)
@@ -300,6 +300,11 @@ EditingStyle::~EditingStyle()
 
 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);
 
index 771c56a..5af08ad 100644 (file)
@@ -239,7 +239,7 @@ void InsertParagraphSeparatorCommand::doApply()
         // Recreate the same structure in the new paragraph.
         
         Vector<Element*> ancestors;
-        getAncestorsInsideBlock(insertionPosition.deprecatedNode(), startBlock, ancestors);      
+        getAncestorsInsideBlock(positionBeforeTabSpan(insertionPosition).deprecatedNode(), startBlock, ancestors);      
         RefPtr<Element> parent = cloneHierarchyUnderNewBlock(ancestors, blockToInsert);
         
         appendBlockPlaceholder(parent);
@@ -254,6 +254,9 @@ void InsertParagraphSeparatorCommand::doApply()
     // similar case where previous position is in another, presumeably nested, block.
     if (isFirstInBlock || !inSameBlock(visiblePos, visiblePos.previous())) {
         Node *refNode;
+        
+        insertionPosition = positionBeforeTabSpan(insertionPosition);
+
         if (isFirstInBlock && !nestNewBlock)
             refNode = startBlock;
         else if (insertionPosition.deprecatedNode() == startBlock && nestNewBlock) {
@@ -270,7 +273,7 @@ void InsertParagraphSeparatorCommand::doApply()
         // Recreate the same structure in the new paragraph.
 
         Vector<Element*> ancestors;
-        getAncestorsInsideBlock(positionAvoidingSpecialElementBoundary(insertionPosition).deprecatedNode(), startBlock, ancestors);
+        getAncestorsInsideBlock(positionAvoidingSpecialElementBoundary(positionBeforeTabSpan(insertionPosition)).deprecatedNode(), startBlock, ancestors);
         
         appendBlockPlaceholder(cloneHierarchyUnderNewBlock(ancestors, blockToInsert));
         
@@ -303,7 +306,7 @@ void InsertParagraphSeparatorCommand::doApply()
 
     // Build up list of ancestors in between the start node and the start block.
     Vector<Element*> ancestors;
-    getAncestorsInsideBlock(insertionPosition.deprecatedNode(), startBlock, ancestors);
+    getAncestorsInsideBlock(positionBeforeTabSpan(insertionPosition).deprecatedNode(), startBlock, ancestors);
 
     // Make sure we do not cause a rendered space to become unrendered.
     // FIXME: We need the affinity for pos, but pos.downstream() does not give it