Navigating to www.apple.com hits assertion in WebCore::TextIteratorCopyableText:...
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 May 2015 22:44:30 +0000 (22:44 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 May 2015 22:44:30 +0000 (22:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=144629
rdar://problem/20689877

Reviewed by Andreas Kling.

This patch ensures that we don't emit empty text for the text iterator.
In TextIterator::handleTextNode before emitting a string, certain characters (\n \t) need to
be replaced with space. When such character is found, we emit the string we've processed so far and
handle the replacement during the next callback.
When the first character in the string needs replacing, there's nothing to emit. However if we don't
handle at least one character, TextIterator::advance believes that processing is done and never calls
TextIterator::handleTextNode back with the rest of the string.

Source/WebCore:

Test: fast/text/simple-line-layout-innerText-with-newline.html

* editing/TextIterator.cpp:
(WebCore::isNewLineOrTabCharacter):
(WebCore::TextIterator::handleTextNode):

LayoutTests:

* fast/text/simple-line-layout-innerText-with-newline-expected.html: Added.
* fast/text/simple-line-layout-innerText-with-newline.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/text/simple-line-layout-innerText-with-newline-expected.html [new file with mode: 0644]
LayoutTests/fast/text/simple-line-layout-innerText-with-newline.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/TextIterator.cpp

index a92f009..dcf6075 100644 (file)
@@ -1,3 +1,22 @@
+2015-05-05  Zalan Bujtas  <zalan@apple.com>
+
+        Navigating to www.apple.com hits assertion in WebCore::TextIteratorCopyableText::set()
+        https://bugs.webkit.org/show_bug.cgi?id=144629
+        rdar://problem/20689877
+
+        Reviewed by Andreas Kling.
+
+        This patch ensures that we don't emit empty text for the text iterator.
+        In TextIterator::handleTextNode before emitting a string, certain characters (\n \t) need to
+        be replaced with space. When such character is found, we emit the string we've processed so far and
+        handle the replacement during the next callback.
+        When the first character in the string needs replacing, there's nothing to emit. However if we don't
+        handle at least one character, TextIterator::advance believes that processing is done and never calls  
+        TextIterator::handleTextNode back with the rest of the string. 
+
+        * fast/text/simple-line-layout-innerText-with-newline-expected.html: Added.
+        * fast/text/simple-line-layout-innerText-with-newline.html: Added.
+
 2015-05-05  Brent Fulgham  <bfulgham@apple.com>
 
         Add overflow scroll-snap tests
diff --git a/LayoutTests/fast/text/simple-line-layout-innerText-with-newline-expected.html b/LayoutTests/fast/text/simple-line-layout-innerText-with-newline-expected.html
new file mode 100644 (file)
index 0000000..121ee16
--- /dev/null
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <title>This tests that innerText works fine with new line characters. (replacing them with space)</title>
+</head>
+<body>
+  <div>foo bar</div>
+</body>
diff --git a/LayoutTests/fast/text/simple-line-layout-innerText-with-newline.html b/LayoutTests/fast/text/simple-line-layout-innerText-with-newline.html
new file mode 100644 (file)
index 0000000..23b1d63
--- /dev/null
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <title>This tests that innerText works fine with new line characters. (replacing them with space)</title>
+</head>
+<body>
+  <div style="font: 0/0 a;">
+    foo
+    bar
+  </div>
+  <div id=result></div>
+  <script>
+    document.getElementById("result").innerHTML = document.body.innerText;
+  </script>
+</body>
index 41471ef..f019102 100644 (file)
@@ -1,3 +1,25 @@
+2015-05-05  Zalan Bujtas  <zalan@apple.com>
+
+        Navigating to www.apple.com hits assertion in WebCore::TextIteratorCopyableText::set()
+        https://bugs.webkit.org/show_bug.cgi?id=144629
+        rdar://problem/20689877
+
+        Reviewed by Andreas Kling.
+
+        This patch ensures that we don't emit empty text for the text iterator.
+        In TextIterator::handleTextNode before emitting a string, certain characters (\n \t) need to
+        be replaced with space. When such character is found, we emit the string we've processed so far and
+        handle the replacement during the next callback.
+        When the first character in the string needs replacing, there's nothing to emit. However if we don't
+        handle at least one character, TextIterator::advance believes that processing is done and never calls  
+        TextIterator::handleTextNode back with the rest of the string. 
+
+        Test: fast/text/simple-line-layout-innerText-with-newline.html
+
+        * editing/TextIterator.cpp:
+        (WebCore::isNewLineOrTabCharacter):
+        (WebCore::TextIterator::handleTextNode):
+
 2015-05-05  Alex Christensen  <achristensen@webkit.org>
 
         [Content Extensions] Use less memory to store the json input.
index b27ac6a..6cf6161 100644 (file)
@@ -515,6 +515,11 @@ static unsigned textNodeOffsetInFlow(const Text& firstTextNodeInRange)
     return textOffset;
 }
 
+static bool isNewLineOrTabCharacter(UChar character)
+{
+    return character == '\n' || character == '\t';
+}
+
 bool TextIterator::handleTextNode()
 {
     Text& textNode = downcast<Text>(*m_node);
@@ -607,16 +612,21 @@ bool TextIterator::handleTextNode()
                 return false;
             }
         }
-        // \n \t single whitespace characters need replacing so that the new line/tab character won't show up.
+        // \n \t single whitespace characters need replacing so that the new line/tab characters don't show up.
         unsigned stopPosition = contentStart;
-        while (stopPosition < contentEnd) {
-            if (rendererText[stopPosition] == '\n' || rendererText[stopPosition] == '\t') {
-                emitText(textNode, renderer, contentStart, stopPosition);
-                m_offset = stopPosition + 1;
-                m_nextRunNeedsWhitespace = true;
+        while (stopPosition < contentEnd && !isNewLineOrTabCharacter(rendererText[stopPosition]))
+            ++stopPosition;
+        // Emit the text up to the new line/tab character.
+        if (stopPosition < contentEnd) {
+            if (stopPosition == contentStart) {
+                emitCharacter(' ', textNode, nullptr, contentStart, contentStart + 1);
+                m_offset = contentStart + 1;
                 return false;
             }
-            ++stopPosition;
+            emitText(textNode, renderer, contentStart, stopPosition);
+            m_offset = stopPosition + 1;
+            m_nextRunNeedsWhitespace = true;
+            return false;
         }
         emitText(textNode, renderer, contentStart, contentEnd);
         // When line ending with collapsed whitespace is present, we need to carry over one whitespace: foo(end of line)bar -> foo bar (otherwise we would end up with foobar).