[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-10851-g50815da
rniwa at webkit.org
rniwa at webkit.org
Wed Dec 22 18:05:27 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit d87f2f644cfec1998b96029b0fa9042b12b01216
Author: rniwa at webkit.org <rniwa at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Tue Dec 7 00:31:34 2010 +0000
2010-12-05 Ryosuke Niwa <rniwa at webkit.org>
Reviewed by Tony Chang.
Executing FormatBlock on multiple paragraphs inside pre does not remove the outer pre
https://bugs.webkit.org/show_bug.cgi?id=47300
The bug was caused by FormatBlockCommand::formatRange's not removing refNode when the refNode
contains more than one paragraphs even when the refNode is fully selected.
Fixed the bug by modifying FormatBlockCommand::formatRange to correctly remove the node in
such a situation.
Also fixed a bug in ApplyBlockElementCommand::formatSelection that the end of selection
is not properly updated when the end of selection resides in the node split by
rangeForParagraphSplittingTextNodesIfNeeded or endOfNextParagrahSplittingTextNodesIfNeeded.
Test: editing/execCommand/format-block-multiple-paragraphs-in-pre.html
* editing/ApplyBlockElementCommand.cpp:
(WebCore::ApplyBlockElementCommand::formatSelection): Calls formatRange with m_endOfLastParagraph.
(WebCore::ApplyBlockElementCommand::rangeForParagraphSplittingTextNodesIfNeeded): Updates
m_endOfLastParagraph when the position points to the node split by this function.
(WebCore::ApplyBlockElementCommand::endOfNextParagrahSplittingTextNodesIfNeeded): Ditto.
* editing/ApplyBlockElementCommand.h: Added m_endOfLastParagraph as a member variable.
* editing/FormatBlockCommand.cpp:
(WebCore::FormatBlockCommand::formatRange): See above.
* editing/FormatBlockCommand.h:
* editing/IndentOutdentCommand.cpp:
(WebCore::IndentOutdentCommand::formatRange): Ignores the end of selection.
* editing/IndentOutdentCommand.h:
2010-12-05 Ryosuke Niwa <rniwa at webkit.org>
Reviewed by Tony Chang.
Executing FormatBlock on multiple paragraphs inside pre does not remove the outer pre
https://bugs.webkit.org/show_bug.cgi?id=47300
Added a test to ensure execCommand('FormatBlock') correctly removes pre when formatting paragraphs within.
Also rebaselined several tests because WebKit no longer erroneously format the paragraphs immediately
after the selection.
* editing/execCommand/format-block-multiple-paragraphs-in-pre-expected.txt: Added.
* editing/execCommand/format-block-multiple-paragraphs-in-pre.html: Added.
* editing/execCommand/format-block-multiple-paragraphs-expected.txt: No longer erroneously format
the last paragraph in a pre by a blockquote when formatting all but the last paragraph in the pre.
* editing/execCommand/indent-pre-expected.txt: No longer erroneously format the last paragraph in
the pre-list by a blockquote.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@73411 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index e71b924..89679b5 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,21 @@
+2010-12-05 Ryosuke Niwa <rniwa at webkit.org>
+
+ Reviewed by Tony Chang.
+
+ Executing FormatBlock on multiple paragraphs inside pre does not remove the outer pre
+ https://bugs.webkit.org/show_bug.cgi?id=47300
+
+ Added a test to ensure execCommand('FormatBlock') correctly removes pre when formatting paragraphs within.
+ Also rebaselined several tests because WebKit no longer erroneously format the paragraphs immediately
+ after the selection.
+
+ * editing/execCommand/format-block-multiple-paragraphs-in-pre-expected.txt: Added.
+ * editing/execCommand/format-block-multiple-paragraphs-in-pre.html: Added.
+ * editing/execCommand/format-block-multiple-paragraphs-expected.txt: No longer erroneously format
+ the last paragraph in a pre by a blockquote when formatting all but the last paragraph in the pre.
+ * editing/execCommand/indent-pre-expected.txt: No longer erroneously format the last paragraph in
+ the pre-list by a blockquote.
+
2010-12-06 Victor Wang <victorw at chromium.org>
Unreviewed.
diff --git a/LayoutTests/editing/execCommand/format-block-multiple-paragraphs-expected.txt b/LayoutTests/editing/execCommand/format-block-multiple-paragraphs-expected.txt
index a524cc8..0bafcd0 100644
--- a/LayoutTests/editing/execCommand/format-block-multiple-paragraphs-expected.txt
+++ b/LayoutTests/editing/execCommand/format-block-multiple-paragraphs-expected.txt
@@ -57,11 +57,10 @@ Formatting:
by p yields:
| "
"
-| <div>
-| <p>
-| "<#selection-anchor>hello"
-| <br>
-| "world<#selection-focus>"
+| <p>
+| "<#selection-anchor>hello"
+| <br>
+| "world<#selection-focus>"
| "
"
@@ -132,8 +131,8 @@ by blockquote yields:
| "<#selection-anchor>hello"
| <br>
| "world<#selection-focus>"
-| <br>
-| "webkit"
+| "webkit
+"
| "
"
diff --git a/LayoutTests/editing/execCommand/format-block-multiple-paragraphs-in-pre-expected.txt b/LayoutTests/editing/execCommand/format-block-multiple-paragraphs-in-pre-expected.txt
new file mode 100644
index 0000000..d442fcf
--- /dev/null
+++ b/LayoutTests/editing/execCommand/format-block-multiple-paragraphs-in-pre-expected.txt
@@ -0,0 +1,75 @@
+This tests ensures formatBlock removes a pre when formatting multiple paragraphs inside the pre.
+
+Formatting all paragraphs by h3 yields:
+| "
+"
+| <h3>
+| "hello"
+| <br>
+| "
+"
+| "world"
+| <br>
+| "
+"
+| "webkit"
+| "
+"
+
+Undo yields:
+| "
+"
+| <pre>
+| "<#selection-anchor>hello
+
+world
+
+webkit<#selection-focus>
+"
+| "
+"
+
+Formatting all but the last paragraph by h3 yields:
+| "
+"
+| <pre>
+| <h3>
+| "<#selection-anchor>hello"
+| "
+"
+| "world"
+| "
+"
+| "<#selection-focus>webkit
+"
+| "
+"
+
+Undo yields:
+| "
+"
+| <pre>
+| "<#selection-anchor>hello
+
+world
+
+<#selection-focus>webkit
+"
+| "
+"
+
+Formatting all but the first paragraph by h3 yields:
+| "
+"
+| <pre>
+| "hello
+"
+| <h3>
+| "
+"
+| "world"
+| "
+"
+| "webkit"
+| "
+"
diff --git a/LayoutTests/editing/execCommand/format-block-multiple-paragraphs-in-pre.html b/LayoutTests/editing/execCommand/format-block-multiple-paragraphs-in-pre.html
new file mode 100644
index 0000000..5ec23ef
--- /dev/null
+++ b/LayoutTests/editing/execCommand/format-block-multiple-paragraphs-in-pre.html
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/dump-as-markup.js"></script>
+<div id="test" contenteditable>
+<pre>
+hello
+
+world
+
+webkit
+</pre>
+</div>
+<script>
+
+Markup.description('This tests ensures formatBlock removes a pre when formatting multiple paragraphs inside the pre.');
+
+var test = document.getElementById('test');
+var original = test.innerHTML;
+window.getSelection().selectAllChildren(test);
+document.execCommand('formatBlock', false, 'h3');
+Markup.dump(test, 'Formatting all paragraphs by h3 yields');
+
+document.execCommand('undo', false, null);
+Markup.dump(test, 'Undo yields');
+window.getSelection().setPosition(test, 0);
+window.getSelection().modify('extend', 'forward', 'line');
+window.getSelection().modify('extend', 'forward', 'line');
+window.getSelection().modify('extend', 'forward', 'line');
+window.getSelection().modify('extend', 'forward', 'line');
+document.execCommand('formatBlock', false, 'h3');
+Markup.dump(test, 'Formatting all but the last paragraph by h3 yields');
+
+document.execCommand('undo', false, null);
+Markup.dump(test, 'Undo yields');
+window.getSelection().setPosition(test, 0);
+window.getSelection().modify('move', 'forward', 'line');
+window.getSelection().modify('extend', 'forward', 'line');
+window.getSelection().modify('extend', 'forward', 'line');
+window.getSelection().modify('extend', 'forward', 'line');
+window.getSelection().modify('extend', 'forward', 'line');
+document.execCommand('formatBlock', false, 'h3');
+Markup.dump(test, 'Formatting all but the first paragraph by h3 yields');
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/editing/execCommand/indent-pre-expected.txt b/LayoutTests/editing/execCommand/indent-pre-expected.txt
index e701721..89736de 100644
--- a/LayoutTests/editing/execCommand/indent-pre-expected.txt
+++ b/LayoutTests/editing/execCommand/indent-pre-expected.txt
@@ -46,8 +46,8 @@ does not crash."
| "list two"
| <br>
| "list three"
-| <br>
-| "list four"
+| "list four
+"
| "
"
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 1999892..c53b9b8 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,35 @@
+2010-12-05 Ryosuke Niwa <rniwa at webkit.org>
+
+ Reviewed by Tony Chang.
+
+ Executing FormatBlock on multiple paragraphs inside pre does not remove the outer pre
+ https://bugs.webkit.org/show_bug.cgi?id=47300
+
+ The bug was caused by FormatBlockCommand::formatRange's not removing refNode when the refNode
+ contains more than one paragraphs even when the refNode is fully selected.
+
+ Fixed the bug by modifying FormatBlockCommand::formatRange to correctly remove the node in
+ such a situation.
+
+ Also fixed a bug in ApplyBlockElementCommand::formatSelection that the end of selection
+ is not properly updated when the end of selection resides in the node split by
+ rangeForParagraphSplittingTextNodesIfNeeded or endOfNextParagrahSplittingTextNodesIfNeeded.
+
+ Test: editing/execCommand/format-block-multiple-paragraphs-in-pre.html
+
+ * editing/ApplyBlockElementCommand.cpp:
+ (WebCore::ApplyBlockElementCommand::formatSelection): Calls formatRange with m_endOfLastParagraph.
+ (WebCore::ApplyBlockElementCommand::rangeForParagraphSplittingTextNodesIfNeeded): Updates
+ m_endOfLastParagraph when the position points to the node split by this function.
+ (WebCore::ApplyBlockElementCommand::endOfNextParagrahSplittingTextNodesIfNeeded): Ditto.
+ * editing/ApplyBlockElementCommand.h: Added m_endOfLastParagraph as a member variable.
+ * editing/FormatBlockCommand.cpp:
+ (WebCore::FormatBlockCommand::formatRange): See above.
+ * editing/FormatBlockCommand.h:
+ * editing/IndentOutdentCommand.cpp:
+ (WebCore::IndentOutdentCommand::formatRange): Ignores the end of selection.
+ * editing/IndentOutdentCommand.h:
+
2010-12-03 Zhenyao Mo <zmo at google.com>
Reviewed by Kenneth Russell.
diff --git a/WebCore/editing/ApplyBlockElementCommand.cpp b/WebCore/editing/ApplyBlockElementCommand.cpp
index ecd3d9b..285650d 100644
--- a/WebCore/editing/ApplyBlockElementCommand.cpp
+++ b/WebCore/editing/ApplyBlockElementCommand.cpp
@@ -109,12 +109,12 @@ void ApplyBlockElementCommand::formatSelection(const VisiblePosition& startOfSel
RefPtr<Element> blockquoteForNextIndent;
VisiblePosition endOfCurrentParagraph = endOfParagraph(startOfSelection);
VisiblePosition endAfterSelection = endOfParagraph(endOfParagraph(endOfSelection).next());
- VisiblePosition endOfLastParagraph = endOfParagraph(endOfSelection);
+ m_endOfLastParagraph = endOfParagraph(endOfSelection).deepEquivalent();
bool atEnd = false;
Position end;
while (endOfCurrentParagraph != endAfterSelection && !atEnd) {
- if (endOfCurrentParagraph == endOfLastParagraph)
+ if (endOfCurrentParagraph.deepEquivalent() == m_endOfLastParagraph)
atEnd = true;
rangeForParagraphSplittingTextNodesIfNeeded(endOfCurrentParagraph, start, end);
@@ -124,7 +124,7 @@ void ApplyBlockElementCommand::formatSelection(const VisiblePosition& startOfSel
Node* enclosingCell = enclosingNodeOfType(start, &isTableCell);
VisiblePosition endOfNextParagraph = endOfNextParagrahSplittingTextNodesIfNeeded(endOfCurrentParagraph, start, end);
- formatRange(start, end, blockquoteForNextIndent);
+ formatRange(start, end, m_endOfLastParagraph, blockquoteForNextIndent);
// Don't put the next paragraph in the blockquote we just created for this paragraph unless
// the next paragraph is in the same cell.
@@ -180,7 +180,11 @@ void ApplyBlockElementCommand::rangeForParagraphSplittingTextNodesIfNeeded(const
end = endOfCurrentParagraph.deepEquivalent();
RenderStyle* startStyle = renderStyleOfEnclosingTextNode(start);
+ bool isStartAndEndOnSameNode = false;
if (startStyle) {
+ isStartAndEndOnSameNode = renderStyleOfEnclosingTextNode(end) && start.node() == end.node();
+ bool isStartAndEndOfLastParagraphOnSameNode = renderStyleOfEnclosingTextNode(m_endOfLastParagraph) && start.node() == m_endOfLastParagraph.node();
+
// Avoid obtanining the start of next paragraph for start
if (startStyle->preserveNewline() && isNewLineAtPosition(start) && !isNewLineAtPosition(start.previous()) && start.offsetInContainerNode() > 0)
start = startOfParagraph(end.previous()).deepEquivalent();
@@ -190,29 +194,44 @@ void ApplyBlockElementCommand::rangeForParagraphSplittingTextNodesIfNeeded(const
int startOffset = start.offsetInContainerNode();
splitTextNode(static_cast<Text*>(start.node()), startOffset);
start = positionBeforeNode(start.node());
- if (start.node() == end.node()) {
+ if (isStartAndEndOnSameNode) {
ASSERT(end.offsetInContainerNode() >= startOffset);
end = Position(end.node(), end.offsetInContainerNode() - startOffset, Position::PositionIsOffsetInAnchor);
}
+ if (isStartAndEndOfLastParagraphOnSameNode) {
+ ASSERT(m_endOfLastParagraph.offsetInContainerNode() >= startOffset);
+ m_endOfLastParagraph = Position(m_endOfLastParagraph.node(), m_endOfLastParagraph.offsetInContainerNode() - startOffset,
+ Position::PositionIsOffsetInAnchor);
+ }
}
}
RenderStyle* endStyle = renderStyleOfEnclosingTextNode(end);
if (endStyle) {
+ bool isEndAndEndOfLastParagraphOnSameNode = renderStyleOfEnclosingTextNode(m_endOfLastParagraph) && end.node() == m_endOfLastParagraph.node();
// Include \n at the end of line if we're at an empty paragraph
if (endStyle->preserveNewline() && start == end
&& end.offsetInContainerNode() < end.containerNode()->maxCharacterOffset()) {
int endOffset = end.offsetInContainerNode();
if (!isNewLineAtPosition(end.previous()) && isNewLineAtPosition(end))
end = Position(end.node(), endOffset + 1, Position::PositionIsOffsetInAnchor);
+ if (isEndAndEndOfLastParagraphOnSameNode && end.offsetInContainerNode() >= m_endOfLastParagraph.offsetInContainerNode())
+ m_endOfLastParagraph = end;
}
// If end is in the middle of a text node, split.
if (!endStyle->collapseWhiteSpace() && end.offsetInContainerNode()
&& end.offsetInContainerNode() < end.containerNode()->maxCharacterOffset()) {
splitTextNode(static_cast<Text*>(end.node()), end.offsetInContainerNode());
- if (start.node() == end.node())
+ if (isStartAndEndOnSameNode)
start = positionBeforeNode(end.node()->previousSibling());
+ if (isEndAndEndOfLastParagraphOnSameNode) {
+ if (m_endOfLastParagraph.offsetInContainerNode() == end.offsetInContainerNode())
+ m_endOfLastParagraph = lastPositionInNode(end.node()->previousSibling());
+ else
+ m_endOfLastParagraph = Position(end.node(), m_endOfLastParagraph.offsetInContainerNode() - end.offsetInContainerNode(),
+ Position::PositionIsOffsetInAnchor);
+ }
end = lastPositionInNode(end.node()->previousSibling());
}
}
@@ -244,6 +263,12 @@ VisiblePosition ApplyBlockElementCommand::endOfNextParagrahSplittingTextNodesIfN
ASSERT(end.offsetInContainerNode() < position.offsetInContainerNode());
end = Position(containerNode->previousSibling(), end.offsetInContainerNode(), Position::PositionIsOffsetInAnchor);
}
+ if (m_endOfLastParagraph.anchorType() == Position::PositionIsOffsetInAnchor && containerNode.get() == m_endOfLastParagraph.containerNode()) {
+ if (m_endOfLastParagraph.offsetInContainerNode() < position.offsetInContainerNode())
+ m_endOfLastParagraph = Position(containerNode->previousSibling(), m_endOfLastParagraph.offsetInContainerNode(), Position::PositionIsOffsetInAnchor);
+ else
+ m_endOfLastParagraph = Position(containerNode, m_endOfLastParagraph.offsetInContainerNode() - 1, Position::PositionIsOffsetInAnchor);
+ }
return Position(containerNode.get(), position.offsetInContainerNode() - 1, Position::PositionIsOffsetInAnchor);
}
diff --git a/WebCore/editing/ApplyBlockElementCommand.h b/WebCore/editing/ApplyBlockElementCommand.h
index 1fc4931..535f499 100644
--- a/WebCore/editing/ApplyBlockElementCommand.h
+++ b/WebCore/editing/ApplyBlockElementCommand.h
@@ -46,13 +46,14 @@ protected:
private:
virtual void doApply();
- virtual void formatRange(const Position& start, const Position&, RefPtr<Element>&) = 0;
+ virtual void formatRange(const Position& start, const Position& end, const Position& endOfSelection, RefPtr<Element>&) = 0;
void rangeForParagraphSplittingTextNodesIfNeeded(const VisiblePosition&, Position&, Position&);
VisiblePosition endOfNextParagrahSplittingTextNodesIfNeeded(VisiblePosition&, Position&, Position&);
QualifiedName m_tagName;
AtomicString m_className;
AtomicString m_inlineStyle;
+ Position m_endOfLastParagraph;
};
}
diff --git a/WebCore/editing/FormatBlockCommand.cpp b/WebCore/editing/FormatBlockCommand.cpp
index 2e42fb3..c40eaa0 100644
--- a/WebCore/editing/FormatBlockCommand.cpp
+++ b/WebCore/editing/FormatBlockCommand.cpp
@@ -58,15 +58,17 @@ void FormatBlockCommand::formatSelection(const VisiblePosition& startOfSelection
m_didApply = true;
}
-void FormatBlockCommand::formatRange(const Position& start, const Position& end, RefPtr<Element>& blockNode)
+void FormatBlockCommand::formatRange(const Position& start, const Position& end, const Position& endOfSelection, RefPtr<Element>& blockNode)
{
Node* nodeToSplitTo = enclosingBlockToSplitTreeTo(start.node());
RefPtr<Node> outerBlock = (start.node() == nodeToSplitTo) ? start.node() : splitTreeToNode(start.node(), nodeToSplitTo);
RefPtr<Node> nodeAfterInsertionPosition = outerBlock;
+ RefPtr<Range> range = Range::create(document(), start, endOfSelection);
Element* refNode = enclosingBlockFlowElement(end);
Element* root = editableRootForPosition(start);
- if (isElementForFormatBlock(refNode->tagQName()) && start == startOfBlock(start) && end == endOfBlock(end)
+ if (isElementForFormatBlock(refNode->tagQName()) && start == startOfBlock(start)
+ && (end == endOfBlock(end) || isNodeVisiblyContainedWithin(refNode, range.get()))
&& refNode != root && !root->isDescendantOf(refNode)) {
// Already in a block element that only contains the current paragraph
if (refNode->hasTagName(tagName()))
diff --git a/WebCore/editing/FormatBlockCommand.h b/WebCore/editing/FormatBlockCommand.h
index 134a422..352fa23 100644
--- a/WebCore/editing/FormatBlockCommand.h
+++ b/WebCore/editing/FormatBlockCommand.h
@@ -45,7 +45,7 @@ private:
FormatBlockCommand(Document*, const QualifiedName& tagName);
void formatSelection(const VisiblePosition& startOfSelection, const VisiblePosition& endOfSelection);
- void formatRange(const Position&, const Position&, RefPtr<Element>&);
+ void formatRange(const Position& start, const Position& end, const Position& endOfSelection, RefPtr<Element>&);
EditAction editingAction() const { return EditActionFormatBlock; }
bool m_didApply;
diff --git a/WebCore/editing/IndentOutdentCommand.cpp b/WebCore/editing/IndentOutdentCommand.cpp
index 7089f6f..13056d3 100644
--- a/WebCore/editing/IndentOutdentCommand.cpp
+++ b/WebCore/editing/IndentOutdentCommand.cpp
@@ -222,7 +222,7 @@ void IndentOutdentCommand::formatSelection(const VisiblePosition& startOfSelecti
outdentRegion(startOfSelection, endOfSelection);
}
-void IndentOutdentCommand::formatRange(const Position& start, const Position& end, RefPtr<Element>& blockquoteForNextIndent)
+void IndentOutdentCommand::formatRange(const Position& start, const Position& end, const Position&, RefPtr<Element>& blockquoteForNextIndent)
{
if (tryIndentingAsListItem(start, end))
blockquoteForNextIndent = 0;
diff --git a/WebCore/editing/IndentOutdentCommand.h b/WebCore/editing/IndentOutdentCommand.h
index 201e794..c28aea3 100644
--- a/WebCore/editing/IndentOutdentCommand.h
+++ b/WebCore/editing/IndentOutdentCommand.h
@@ -53,7 +53,7 @@ private:
void indentIntoBlockquote(const Position&, const Position&, RefPtr<Element>&);
void formatSelection(const VisiblePosition& startOfSelection, const VisiblePosition& endOfSelection);
- void formatRange(const Position&, const Position&, RefPtr<Element>& blockquoteForNextIndent);
+ void formatRange(const Position& start, const Position& end, const Position& endOfSelection, RefPtr<Element>& blockquoteForNextIndent);
EIndentType m_typeOfAction;
int m_marginInPixels;
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list