diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 7eb16488bccd9798ea67adb42b67fa5865ff1b21..4d0be3a3d08541b3e3aa9a00d5f1dfa0bc3e3e98 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,19 @@ +2011-04-07 Ryosuke Niwa + + 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 Reviewed by Eric Seidel. @@ -408,6 +424,19 @@ * 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 + + 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 Reviewed by Eric Seidel. diff --git a/LayoutTests/editing/inserting/insert-div-021-expected.txt b/LayoutTests/editing/inserting/insert-div-021-expected.txt index 47f4d03290583282e38e51698038b9a782f3a4e0..2c756a27c01f9f433f235820f8df072dadc3c42f 100644 --- a/LayoutTests/editing/inserting/insert-div-021-expected.txt +++ b/LayoutTests/editing/inserting/insert-div-021-expected.txt @@ -32,7 +32,5 @@ Fix for this bug: REGRESSION (Mail): After deleting, hi |
|
|
-| -| id="test" | "<#selection-caret>bar " diff --git a/LayoutTests/editing/inserting/insert-paragraph-after-tab-span-and-text-expected.txt b/LayoutTests/editing/inserting/insert-paragraph-after-tab-span-and-text-expected.txt new file mode 100644 index 0000000000000000000000000000000000000000..c8c813c4d22420b7cf17133c9f090d226a03ff50 --- /dev/null +++ b/LayoutTests/editing/inserting/insert-paragraph-after-tab-span-and-text-expected.txt @@ -0,0 +1,19 @@ +This test ensures WebKit inserts a paragraph separator properly at the end of a tab span. +| " +" +| +| class="Apple-tab-span" +| style="white-space:pre" +| " hello" +|
+| <#selection-caret> +|
+| " +" +| +| class="Apple-style-span" +| +| class="Apple-tab-span" +| style="white-space: pre; " +| " " +| "world" diff --git a/LayoutTests/editing/inserting/insert-paragraph-after-tab-span-and-text.html b/LayoutTests/editing/inserting/insert-paragraph-after-tab-span-and-text.html new file mode 100644 index 0000000000000000000000000000000000000000..fc7e3079381974ef90fb1b8f6068e08431a41467 --- /dev/null +++ b/LayoutTests/editing/inserting/insert-paragraph-after-tab-span-and-text.html @@ -0,0 +1,20 @@ + + + +
+ hello
+ world
+ + + + diff --git a/LayoutTests/editing/inserting/insert-paragraph-at-end-of-line-expected.txt b/LayoutTests/editing/inserting/insert-paragraph-at-end-of-line-expected.txt index dfede3b743536f6f51d50d10063371f252581241..a5001e9cea27078adba04eb0f69ec30f15286f57 100644 --- a/LayoutTests/editing/inserting/insert-paragraph-at-end-of-line-expected.txt +++ b/LayoutTests/editing/inserting/insert-paragraph-at-end-of-line-expected.txt @@ -18,9 +18,6 @@ If the test has passed, the numbers should be in order, and only "1" should be a | id="anchor" | "1" |
-| -| href="#" -| id="anchor" | "2<#selection-caret>" |
|
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 index 0000000000000000000000000000000000000000..0d904704f8bbeacf3cfc7947c763a58bb983f75f --- /dev/null +++ b/LayoutTests/editing/inserting/insert-paragraph-separator-tab-span-expected.txt @@ -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: +| +| class="Apple-tab-span" +| style="white-space:pre" +| " " +|
+| "hello world WebKit <#selection-caret>" + +first visible position: +|
+|
+| "hello world WebKit <#selection-caret>" +| +| class="Apple-tab-span" +| style="white-space:pre" +| " " + +before tab span: +| "hi, " +|
+| "hello world WebKit <#selection-caret>" +| +| class="Apple-tab-span" +| style="white-space:pre" +| " " +| " rocks" + +after tab span: +| "hi, " +| +| class="Apple-tab-span" +| style="white-space:pre" +| " " +|
+| "hello world WebKit <#selection-caret>rocks" 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 index 0000000000000000000000000000000000000000..406ddb7dfec3ae9b2c6fbd7904813bab5ebf948a --- /dev/null +++ b/LayoutTests/editing/inserting/insert-paragraph-separator-tab-span.html @@ -0,0 +1,32 @@ + + + +
+
+
hi, rocks
+
hi, rocks
+ + + + diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index d8daaac806967e15f2e85c4cfebe6ffbece0440f..22c6f147d22c02cf495e1a164ffcc2d8c38a0468 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,37 @@ +2011-04-07 Ryosuke Niwa + + 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 Reviewed by Darin Adler. diff --git a/Source/WebCore/editing/DeleteSelectionCommand.cpp b/Source/WebCore/editing/DeleteSelectionCommand.cpp index af8473fae6e777d95be516bf388c38017b67e68f..529c71d747ea4182aff7ed4b65ad52fd32d224bb 100644 --- a/Source/WebCore/editing/DeleteSelectionCommand.cpp +++ b/Source/WebCore/editing/DeleteSelectionCommand.cpp @@ -281,7 +281,7 @@ void DeleteSelectionCommand::saveTypingStyleState() return; // 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())); // If we're deleting into a Mail blockquote, save the style at end() instead of start() diff --git a/Source/WebCore/editing/EditingStyle.cpp b/Source/WebCore/editing/EditingStyle.cpp index 668c9431edd8fa9b986b5f71491869853eb9bcdd..b2195a9a69f703db0d51b7fb38b0c8887aff7956 100644 --- a/Source/WebCore/editing/EditingStyle.cpp +++ b/Source/WebCore/editing/EditingStyle.cpp @@ -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 computedStyleAtPosition = computedStyle(node); m_mutableStyle = propertiesToInclude == AllProperties && computedStyleAtPosition ? computedStyleAtPosition->copy() : editingStyleFromComputedStyle(computedStyleAtPosition); diff --git a/Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp b/Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp index 771c56a75bc21dd60ee04f53a9daf8551abe7b93..2b7ee8cea3f362bbc60154999a94b83af0056002 100644 --- a/Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp +++ b/Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp @@ -239,7 +239,7 @@ void InsertParagraphSeparatorCommand::doApply() // Recreate the same structure in the new paragraph. Vector ancestors; - getAncestorsInsideBlock(insertionPosition.deprecatedNode(), startBlock, ancestors); + getAncestorsInsideBlock(positionOutsideTabSpan(insertionPosition).deprecatedNode(), startBlock, ancestors); RefPtr 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 = positionOutsideTabSpan(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 ancestors; - getAncestorsInsideBlock(positionAvoidingSpecialElementBoundary(insertionPosition).deprecatedNode(), startBlock, ancestors); + getAncestorsInsideBlock(positionAvoidingSpecialElementBoundary(positionOutsideTabSpan(insertionPosition)).deprecatedNode(), startBlock, ancestors); appendBlockPlaceholder(cloneHierarchyUnderNewBlock(ancestors, blockToInsert)); @@ -299,11 +302,7 @@ void InsertParagraphSeparatorCommand::doApply() // At this point, the insertionPosition's node could be a container, and we want to make sure we include // 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 ancestors; - getAncestorsInsideBlock(insertionPosition.deprecatedNode(), startBlock, ancestors); + insertionPosition = positionOutsideTabSpan(VisiblePosition(insertionPosition).deepEquivalent()); // 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 @@ -335,48 +334,35 @@ void InsertParagraphSeparatorCommand::doApply() insertNodeAfter(blockToInsert.get(), startBlock); updateLayout(); - - // Make clones of ancestors in between the start node and the outer block. - RefPtr parent = cloneHierarchyUnderNewBlock(ancestors, blockToInsert); // If the paragraph separator was inserted at the end of a paragraph, an empty line must be // created. All of the nodes, starting at visiblePos, are about to be added to the new paragraph // element. If the first node to be inserted won't be one that will hold an empty line open, add a br. 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 if (splitText) { updateLayout(); diff --git a/Source/WebCore/editing/htmlediting.cpp b/Source/WebCore/editing/htmlediting.cpp index 564eff6e14ab5dcc608ca852ec5e5986916cbcf8..bdd92bdb398736e13f99ff0aaa3307a31b32d4fc 100644 --- a/Source/WebCore/editing/htmlediting.cpp +++ b/Source/WebCore/editing/htmlediting.cpp @@ -894,14 +894,17 @@ bool isNodeInTextFormControl(Node* node) return ancestor->isElementNode() && static_cast(ancestor)->isTextFormControl(); } -Position positionBeforeTabSpan(const Position& pos) +Position positionOutsideTabSpan(const Position& pos) { - Node* node = pos.deprecatedNode(); + 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); } diff --git a/Source/WebCore/editing/htmlediting.h b/Source/WebCore/editing/htmlediting.h index cb2b2a429f9e1eee797ecba9fc700d6f287fe247..b7c2bf7b5caffd18abfc894e12f61b1b64615c2a 100644 --- a/Source/WebCore/editing/htmlediting.h +++ b/Source/WebCore/editing/htmlediting.h @@ -111,7 +111,7 @@ Position previousCandidate(const Position&); Position nextVisuallyDistinctCandidate(const Position&); 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); Position positionOutsideContainingSpecialElement(const Position&, Node** containingSpecialElement=0);