Chromium Code Reviews Chromium Code Reviews
Issue 178543013:
Fix a bug in Range::didSplitTextNode() that may yield an invalid Range object. (Closed)
Base URL: svn://svn.chromium.org/blink/trunk| Index: LayoutTests/fast/dom/Range/surroundContents-crash.html |
| diff --git a/LayoutTests/fast/dom/Range/surroundContents-crash.html b/LayoutTests/fast/dom/Range/surroundContents-crash.html |
| new file mode 100644 |
| index 0000000000000000000000000000000000000000..b480d641d670290f68d504c46ec6a0bd1c7950aa |
| --- /dev/null |
| +++ b/LayoutTests/fast/dom/Range/surroundContents-crash.html |
| @@ -0,0 +1,70 @@ |
| +<!DOCTYPE html> |
| +<html> |
| +<head> |
| +<title>Range.surroundContents() crash</title> |
| +<script src="../../../resources/js-test.js"></script> |
| +</head> |
| +<body> |
| +<script> |
| +description('Range::didSplitTextNode() should not yield an invalid Range object nor cause a crash inside surroundContents().'); |
| + |
| +window.jsTestIsAsync = true; |
| + |
| +var range; |
| +var textContainer; |
| +var textToBeSplit; |
| +var newTextNode; |
| + |
| +function run() |
| +{ |
| + textContainer = document.createElement('div'); |
| + textToBeSplit = document.createTextNode('SPLITME'); |
| + textContainer.appendChild(textToBeSplit); |
| + document.body.appendChild(textContainer); |
| + |
| + var surroundParent = document.createElement('div'); |
| + var textToBeRemoved = document.createTextNode('I will be removed.'); |
| + surroundParent.appendChild(textToBeRemoved); |
| + document.body.appendChild(surroundParent); |
| + |
| + // Range.surroundContents(newParent) removes newParent's children during its preprocess phase, thus |
| + // the following event handler is called in the middle of surroundContents() method. |
| + textToBeRemoved.addEventListener('DOMNodeRemoved', function (event) { |
|
yosin_UTC9
2014/03/04 09:26:29
nit: Can we use DOM mutation observer instead of i
Yuta Kitamura
2014/03/05 01:15:53
This crash needs synchronous call within surroundC
|
| + shouldEvaluateTo('textContainer.childNodes.length', 1); |
| + shouldBeTrue('range.startContainer === textToBeSplit'); |
| + shouldEvaluateTo('range.startOffset', textToBeSplit.length); |
| + shouldBeTrue('range.endContainer === textContainer'); |
| + shouldEvaluateTo('range.endOffset', 1); |
| + |
| + // A bug in Range::didSplitTextNode() yielded an invalid Range object (m_start is located *after* m_end). |
| + // This leads to a crash if this happens within surroundContents(). |
| + textToBeSplit.splitText(textToBeSplit.length - 1); |
| + newTextNode = textToBeSplit.nextSibling; |
| + |
| + // To reproduce a crash, there must be something in between split text nodes. |
| + textContainer.insertBefore(document.createElement('span'), newTextNode); |
| + |
| + shouldEvaluateTo('textContainer.childNodes.length', 3); |
| + shouldBeTrue('range.startContainer === newTextNode'); |
| + shouldEvaluateTo('range.startOffset', newTextNode.length); |
| + shouldBeTrue('range.endContainer === textContainer'); |
| + shouldEvaluateTo('range.endOffset', 3); |
| + }); |
| + |
| + range = new Range(); |
| + range.setStart(textToBeSplit, textToBeSplit.length); |
| + range.setEnd(textContainer, 1); |
| + range.surroundContents(surroundParent); |
| + |
| + testPassed('Did not crash.'); |
| + |
| + // Cleanup. |
| + document.body.removeChild(textContainer); |
| + |
| + window.finishJSTest(); |
| +} |
| + |
| +window.setTimeout(run, 0); |
| +</script> |
| +</body> |
| +</html> |
