Attachment 34883 Details for Bug 28306 – patch update 1

[patch] patch update 1

28306.diff (text/plain), 9.34 KB, created by MORITA Hajime on 2009-08-14 19:49:41 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: MORITA Hajime

Created: 2009-08-14 19:49:41 PDT

Size: 9.34 KB

patch

obsolete

>diff --git a/LayoutTests/.DS_Store b/LayoutTests/.DS_Store
>new file mode 100644
>index 0000000..fa50714
>Binary files /dev/null and b/LayoutTests/.DS_Store differ
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index c48f987..879e566 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,22 @@
>+2009-08-14  MORITA Hajime  <morrita@gmail.com>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Bug 28306: double-clicking a word inside <b> beside newline select two words
>+	https://bugs.webkit.org/show_bug.cgi?id=28306
>+
>+	SimplifiedBackwardsTextIterator missed trailing whitespaces just
>+	before folding line-break, which is used to detect word
>+	boundaries. This change fix to use RenderText::text().length() to
>+	extract text from render objects because RenderText::text() contains
>+	trailing whitespaces egardless of its layout result.
>+	
>+	original code uses InlineTextBox::len() via caretMaxOffset(),
>+	which does not contains trailing whitespaces.
>+	
>+        * editing/selection/doubleclick-beside-cr-span-expected.txt: Added.
>+        * editing/selection/doubleclick-beside-cr-span.html: Added.
>+
> 2009-08-13  Ariya Hidayat  <ariya.hidayat@nokia.com>
> 
>         Rubber-stamped by George Staikos.
>diff --git a/LayoutTests/editing/.DS_Store b/LayoutTests/editing/.DS_Store
>new file mode 100644
>index 0000000..80e8fb1
>Binary files /dev/null and b/LayoutTests/editing/.DS_Store differ
>diff --git a/LayoutTests/editing/selection/doubleclick-beside-cr-span-expected.txt b/LayoutTests/editing/selection/doubleclick-beside-cr-span-expected.txt
>new file mode 100644
>index 0000000..ed743aa
>--- /dev/null
>+++ b/LayoutTests/editing/selection/doubleclick-beside-cr-span-expected.txt
>@@ -0,0 +1,19 @@
>+This tests that double-clicking a word that follows newline and span
>+
>+minimum case
>+
>+abcd efgh select1
>+with another word after the span
>+
>+abcd efgh ijkl mnop select2 nottoselect
>+with another word in same the span
>+
>+abcd efgh ijkl mnop select3 not notyet
>+with another word before the span, in the same line
>+
>+abcd efgh ijkl mnop qrst select4 notyet
>+Passed totest1
>+Passed totest2
>+Passed totest3
>+Passed totest4
>+
>diff --git a/LayoutTests/editing/selection/doubleclick-beside-cr-span.html b/LayoutTests/editing/selection/doubleclick-beside-cr-span.html
>new file mode 100644
>index 0000000..2c712d7
>--- /dev/null
>+++ b/LayoutTests/editing/selection/doubleclick-beside-cr-span.html
>@@ -0,0 +1,107 @@
>+<html>
>+<head>
>+<script>
>+if (window.layoutTestController) {
>+     layoutTestController.dumpAsText();
>+     layoutTestController.setSmartInsertDeleteEnabled(false);
>+     layoutTestController.setSelectTrailingWhitespaceEnabled(true);
>+}
>+
>+function getPositionOfNode(id)
>+{
>+    var n = document.getElementById(id);
>+    var pos = {x: 0, y: 0};
>+
>+    while (n) {
>+        pos.x += n.offsetLeft + n.clientLeft;
>+        pos.y += n.offsetTop + n.clientTop;
>+        n = n.offsetParent;
>+    }
>+    return pos;
>+}
>+
>+function doubleClickNode(id)
>+{
>+    var pos = getPositionOfNode(id);
>+    eventSender.mouseMoveTo(pos.x + 2, pos.y + 2);
>+    eventSender.mouseDown();
>+    eventSender.leapForward(1);
>+    eventSender.mouseUp();
>+    eventSender.leapForward(100);
>+    eventSender.mouseDown();
>+    eventSender.leapForward(1);
>+    eventSender.mouseUp();
>+}
>+
>+function doTest(testId, expectedText)
>+{
>+    // Simulate a double click.
>+    doubleClickNode(testId);
>+
>+    // Get the text of the current selection.
>+    var sel = window.getSelection();
>+    var actualText = sel.getRangeAt(0).toString();
>+
>+    if (expectedText == actualText) {
>+        log("Passed " + testId);
>+    } else {
>+        log("Failed " + testId);
>+        log("  Expected: " + expectedText);
>+        log("  Actual: " + actualText);
>+    }
>+
>+}
>+
>+function log(msg)
>+{
>+    var l = document.getElementById('log');
>+    l.appendChild(document.createTextNode(msg));
>+    l.appendChild(document.createElement('br'));
>+}
>+
>+function runTests()
>+{
>+    if (window.layoutTestController) {
>+        doTest("totest1", "select1");
>+        // trailing whitespaces are ugly, but these are preexisting 
>+        // behaviour - although could be filed as a bug.
>+        doTest("totest2", "select2 ");
>+        doTest("totest3", "select3 ");
>+        doTest("totest4", "select4 ");
>+    }
>+}
>+
>+
>+</script>
>+</head>
>+<body onload="runTests()">
>+
>+<p>
>+This tests that double-clicking a word that follows newline and span
>+</p>
>+
>+<h3>minimum case</h3>
>+<div style="width:10pt">
>+abcd efgh <b id="totest1">select1</b>
>+</div>
>+
>+<h3>with another word after the span</h3>
>+<div style="width:100pt">
>+abcd efgh ijkl mnop <b id="totest2">select2</b> nottoselect
>+</div>
>+
>+<h3>with another word in same the span</h3>
>+<div style="width:100pt">
>+abcd efgh ijkl mnop <b id="totest3">select3 not</b> notyet
>+</div>
>+
>+<h3>with another word before the span, in the same line</h3>
>+<div style="width:100pt">
>+abcd efgh ijkl mnop qrst <b id="totest4">select4</b> notyet
>+</div>
>+
>+<pre id="log">
>+</pre>
>+
>+</body>
>+</html>
>diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
>index 5bee59b..dc0ee72 100644
>--- a/WebCore/ChangeLog
>+++ b/WebCore/ChangeLog
>@@ -1,3 +1,31 @@
>+2009-08-14  MORITA Hajime  <morrita@gmail.com>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Bug 28306: double-clicking a word inside <b> beside newline select two words
>+	https://bugs.webkit.org/show_bug.cgi?id=28306
>+
>+	SimplifiedBackwardsTextIterator missed trailing whitespaces just
>+	before folding line-break, which is used to detect word
>+	boundaries. This change fix to use RenderText::text().length() to
>+	extract text from render objects because RenderText::text() contains
>+	trailing whitespaces egardless of its layout result.
>+	
>+	original code uses InlineTextBox::len() via caretMaxOffset(),
>+	which does not contains trailing whitespaces.
>+	
>+        Test: editing/selection/doubleclick-beside-cr-span.html
>+
>+        * editing/TextIterator.cpp:
>+        (WebCore::SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator):
>+	remove SimplifiedBackwardsTextIterator::m_offset
>+        (WebCore::SimplifiedBackwardsTextIterator::advance):
>+        (WebCore::SimplifiedBackwardsTextIterator::handleTextNode):
>+	
>+        * editing/TextIterator.h: remove
>+	  SimplifiedBackwardsTextIterator::m_offset declaration, whose
>+	  value is now computed on demand.
>+
> 2009-08-13  Mark Rowe  <mrowe@apple.com>
> 
>         Revert r47185, the fix for <https://bugs.webkit.org/show_bug.cgi?id=28185>, as it broke the
>diff --git a/WebCore/editing/TextIterator.cpp b/WebCore/editing/TextIterator.cpp
>index 08fda16..7d74493 100644
>--- a/WebCore/editing/TextIterator.cpp
>+++ b/WebCore/editing/TextIterator.cpp
>@@ -947,7 +947,6 @@ SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator(const Range* r)
> 
>     m_node = endNode;
>     setUpFullyClippedStack(m_fullyClippedStack, m_node);    
>-    m_offset = endOffset;
>     m_handledNode = false;
>     m_handledChildren = endOffset == 0;
> 
>@@ -982,11 +981,11 @@ void SimplifiedBackwardsTextIterator::advance()
>             RenderObject* renderer = m_node->renderer();
>             if (renderer && renderer->isText() && m_node->nodeType() == Node::TEXT_NODE) {
>                 // FIXME: What about CDATA_SECTION_NODE?
>-                if (renderer->style()->visibility() == VISIBLE && m_offset > 0)
>+                if (renderer->style()->visibility() == VISIBLE)
>                     m_handledNode = handleTextNode();
>             } else if (renderer && (renderer->isImage() || renderer->isWidget())) {
>-                if (renderer->style()->visibility() == VISIBLE && m_offset > 0)
>-                    m_handledNode = handleReplacedElement();
>+                if (renderer->style()->visibility() == VISIBLE)
>+                    m_handledNode = handleReplacedElement(); 
>             } else
>                 m_handledNode = handleNonTextNode();
>             if (m_positionNode)
>@@ -1030,7 +1029,6 @@ void SimplifiedBackwardsTextIterator::advance()
>         m_node = next;
>         if (m_node)
>             pushFullyClippedState(m_fullyClippedStack, m_node);
>-        m_offset = m_node ? caretMaxOffset(m_node) : 0;
>         m_handledNode = false;
>         m_handledChildren = false;
>         
>@@ -1049,11 +1047,15 @@ bool SimplifiedBackwardsTextIterator::handleTextNode()
>     if (!renderer->firstTextBox() && str.length() > 0)
>         return true;
> 
>-    m_positionEndOffset = m_offset;
>+    // use str.length() instead of caretMaxOffset()
>+    // because caretMaxOffset() is based on InlineBox and not considering trailing whitespaces,
>+    // which should be handled during word-boundary detection.
>+    m_positionEndOffset = (m_node == m_endNode) ? m_endOffset : str.length();
>+    if (m_positionEndOffset == 0)
>+      return false;
> 
>-    m_offset = (m_node == m_startNode) ? m_startOffset : 0;
>     m_positionNode = m_node;
>-    m_positionStartOffset = m_offset;
>+    m_positionStartOffset = (m_node == m_startNode) ? m_startOffset : 0;
>     m_textLength = m_positionEndOffset - m_positionStartOffset;
>     m_textCharacters = str.characters() + m_positionStartOffset;
> 
>diff --git a/WebCore/editing/TextIterator.h b/WebCore/editing/TextIterator.h
>index 44af3e5..b6b20f4 100644
>--- a/WebCore/editing/TextIterator.h
>+++ b/WebCore/editing/TextIterator.h
>@@ -175,7 +175,6 @@ private:
>     // Current position, not necessarily of the text being returned, but position
>     // as we walk through the DOM tree.
>     Node* m_node;
>-    int m_offset;
>     bool m_handledNode;
>     bool m_handledChildren;
>     BitStack m_fullyClippedStack;

Actions: View | Formatted Diff | Diff

Attachments on bug 28306: 34845 | 34847 | 34883 | 35046 | 43924 | 44748 | 44874 | 45030