<http://webkit.org/b/129751>
<rdar://problem/
16237965>
Reviewed by Jon Honeycutt.
Merged from Blink (patch by Yuta Kitamura):
https://src.chromium.org/viewvc/blink?revision=168160&view=revision
http://crbug.com/345005
The root cause is CompositeEditCommand::moveParagraphWithClones() passing
two positions |start| and |end| which do not follow the document order,
i.e. in some situations |start| is located after |end| because of
the difference in affinity.
This patch fixes this crash by normalizing |end| to |start| in such situations.
It also adds an ASSERT that checks the relationship between |start| and |end|.
Source/WebCore:
Test: editing/execCommand/format-block-crash.html
* editing/CompositeEditCommand.cpp:
(WebCore::CompositeEditCommand::cloneParagraphUnderNewElement):
(WebCore::CompositeEditCommand::moveParagraphWithClones):
* editing/CompositeEditCommand.h:
LayoutTests:
* editing/execCommand/format-block-crash-expected.txt: Added.
* editing/execCommand/format-block-crash.html: Added.
* editing/execCommand/resources/format-block-crash-iframe.html: Added.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@165138
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2014-03-05 David Kilzer <ddkilzer@apple.com>
+
+ Fix crash in CompositeEditCommand::cloneParagraphUnderNewElement()
+ <http://webkit.org/b/129751>
+ <rdar://problem/16237965>
+
+ Reviewed by Jon Honeycutt.
+
+ Merged from Blink (patch by Yuta Kitamura):
+ https://src.chromium.org/viewvc/blink?revision=168160&view=revision
+ http://crbug.com/345005
+
+ The root cause is CompositeEditCommand::moveParagraphWithClones() passing
+ two positions |start| and |end| which do not follow the document order,
+ i.e. in some situations |start| is located after |end| because of
+ the difference in affinity.
+
+ This patch fixes this crash by normalizing |end| to |start| in such situations.
+ It also adds an ASSERT that checks the relationship between |start| and |end|.
+
+ * editing/execCommand/format-block-crash-expected.txt: Added.
+ * editing/execCommand/format-block-crash.html: Added.
+ * editing/execCommand/resources/format-block-crash-iframe.html: Added.
+
2014-03-05 Radu Stavila <stavila@adobe.com>
[CSS Regions] Scrollable regions
--- /dev/null
+Should not crash if we load a test case from crbug.com/345005.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS event.data is "FINISH"
+PASS Did not crash.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
--- /dev/null
+<!DOCTYPE html>
+<html>
+<head>
+<title>FormatBlock crash</title>
+<script src="../../resources/js-test.js"></script>
+</head>
+<body>
+<script>
+description('Should not crash if we load a test case from crbug.com/345005.');
+
+window.jsTestIsAsync = true;
+
+window.addEventListener('message', didReceiveMessage, false);
+
+var iframe = document.createElement('iframe');
+iframe.src = 'resources/format-block-crash-iframe.html';
+document.body.appendChild(iframe);
+
+function didReceiveMessage(event)
+{
+ shouldBeEqualToString('event.data', 'FINISH');
+ document.body.removeChild(iframe);
+ testPassed('Did not crash.');
+ window.finishJSTest();
+}
+</script>
+</body>
+</html>
--- /dev/null
+<!DOCTYPE html>
+<html>
+<head>
+<title>FormatBlock crash test case</title>
+</head>
+<!-- This is a minified version of the clusterfuzz test case at https://code.google.com/p/chromium/issues/detail?id=345005 -->
+<body style="display: table-row;">
+<script>
+function run()
+{
+ document.designMode = 'on';
+ document.execCommand('SelectAll');
+ document.execCommand('FormatBlock', false, '<' + 'div>');
+ window.setTimeout(notifyFinish, 0);
+}
+
+function notifyFinish()
+{
+ window.parent.postMessage('FINISH', '*');
+}
+
+window.setTimeout(run, 0);
+</script>
+<span contenteditable="true" style="display: table-caption;"></span>
+<span></span>
+<div style="display: -webkit-inline-box;"><span><span>B</span></span></div>
+<div></div>
+</body>
+</html>
+2014-03-05 David Kilzer <ddkilzer@apple.com>
+
+ Fix crash in CompositeEditCommand::cloneParagraphUnderNewElement()
+ <http://webkit.org/b/129751>
+ <rdar://problem/16237965>
+
+ Reviewed by Jon Honeycutt.
+
+ Merged from Blink (patch by Yuta Kitamura):
+ https://src.chromium.org/viewvc/blink?revision=168160&view=revision
+ http://crbug.com/345005
+
+ The root cause is CompositeEditCommand::moveParagraphWithClones() passing
+ two positions |start| and |end| which do not follow the document order,
+ i.e. in some situations |start| is located after |end| because of
+ the difference in affinity.
+
+ This patch fixes this crash by normalizing |end| to |start| in such situations.
+ It also adds an ASSERT that checks the relationship between |start| and |end|.
+
+ Test: editing/execCommand/format-block-crash.html
+
+ * editing/CompositeEditCommand.cpp:
+ (WebCore::CompositeEditCommand::cloneParagraphUnderNewElement):
+ (WebCore::CompositeEditCommand::moveParagraphWithClones):
+ * editing/CompositeEditCommand.h:
+
2014-03-05 Radu Stavila <stavila@adobe.com>
[CSS Regions] Scrollable regions
// Clone the paragraph between start and end under blockElement,
// preserving the hierarchy up to outerNode.
-void CompositeEditCommand::cloneParagraphUnderNewElement(Position& start, Position& end, Node* passedOuterNode, Element* blockElement)
+void CompositeEditCommand::cloneParagraphUnderNewElement(const Position& start, const Position& end, Node* passedOuterNode, Element* blockElement)
{
+ ASSERT(comparePositions(start, end) <= 0);
+
// First we clone the outerNode
RefPtr<Node> lastNode;
RefPtr<Node> outerNode = passedOuterNode;
// We upstream() the end and downstream() the start so that we don't include collapsed whitespace in the move.
// When we paste a fragment, spaces after the end and before the start are treated as though they were rendered.
Position start = startOfParagraphToMove.deepEquivalent().downstream();
- Position end = endOfParagraphToMove.deepEquivalent().upstream();
+ Position end = startOfParagraphToMove == endOfParagraphToMove ? start : endOfParagraphToMove.deepEquivalent().upstream();
cloneParagraphUnderNewElement(start, end, outerNode, blockElement);
void moveParagraph(const VisiblePosition&, const VisiblePosition&, const VisiblePosition&, bool preserveSelection = false, bool preserveStyle = true);
void moveParagraphs(const VisiblePosition&, const VisiblePosition&, const VisiblePosition&, bool preserveSelection = false, bool preserveStyle = true);
void moveParagraphWithClones(const VisiblePosition& startOfParagraphToMove, const VisiblePosition& endOfParagraphToMove, Element* blockElement, Node* outerNode);
- void cloneParagraphUnderNewElement(Position& start, Position& end, Node* outerNode, Element* blockElement);
+ void cloneParagraphUnderNewElement(const Position& start, const Position& end, Node* outerNode, Element* blockElement);
void cleanupAfterDeletion(VisiblePosition destination = VisiblePosition());
bool breakOutOfEmptyListItem();