Title: [183835] trunk
Revision
183835
Author
[email protected]
Date
2015-05-05 15:44:30 -0700 (Tue, 05 May 2015)

Log Message

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.

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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (183834 => 183835)


--- trunk/LayoutTests/ChangeLog	2015-05-05 22:31:00 UTC (rev 183834)
+++ trunk/LayoutTests/ChangeLog	2015-05-05 22:44:30 UTC (rev 183835)
@@ -1,3 +1,22 @@
+2015-05-05  Zalan Bujtas  <[email protected]>
+
+        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  <[email protected]>
 
         Add overflow scroll-snap tests

Added: trunk/LayoutTests/fast/text/simple-line-layout-innerText-with-newline-expected.html (0 => 183835)


--- trunk/LayoutTests/fast/text/simple-line-layout-innerText-with-newline-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/simple-line-layout-innerText-with-newline-expected.html	2015-05-05 22:44:30 UTC (rev 183835)
@@ -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>

Added: trunk/LayoutTests/fast/text/simple-line-layout-innerText-with-newline.html (0 => 183835)


--- trunk/LayoutTests/fast/text/simple-line-layout-innerText-with-newline.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/simple-line-layout-innerText-with-newline.html	2015-05-05 22:44:30 UTC (rev 183835)
@@ -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>

Modified: trunk/Source/WebCore/ChangeLog (183834 => 183835)


--- trunk/Source/WebCore/ChangeLog	2015-05-05 22:31:00 UTC (rev 183834)
+++ trunk/Source/WebCore/ChangeLog	2015-05-05 22:44:30 UTC (rev 183835)
@@ -1,3 +1,25 @@
+2015-05-05  Zalan Bujtas  <[email protected]>
+
+        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  <[email protected]>
 
         [Content Extensions] Use less memory to store the json input.

Modified: trunk/Source/WebCore/editing/TextIterator.cpp (183834 => 183835)


--- trunk/Source/WebCore/editing/TextIterator.cpp	2015-05-05 22:31:00 UTC (rev 183834)
+++ trunk/Source/WebCore/editing/TextIterator.cpp	2015-05-05 22:44:30 UTC (rev 183835)
@@ -515,6 +515,11 @@
     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 @@
                 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).
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to