Changeset 164180 in webkit

Timestamp:

Feb 15, 2014 2:32:05 PM (10 years ago)

Author:

rniwa@webkit.org

Message:

computeSelectionStart and computeSelectionEnd shouldn't trigger synchronous layout
​https://bugs.webkit.org/show_bug.cgi?id=128806

Reviewed by Darin Adler.

Added indexForPosition to HTMLTextFormControlElement. Like r163825, this patch traverses the DOM tree
instead of the render tree to compute the index for a given position.

We traverse the DOM Tree backwards starting at the specified Position all the way back to the beginning
of the inner text element. The index is computed as the number of characters we encountered during
this backwards DOM traversal.

It's worth noting that passedPosition.computeNodeBeforePosition() returns and only returns 0 when the
position is before the first node of its parent or inside a text node. In such cases, we call
passedPosition.containerNode() to find the parent or the text node.

• html/HTMLTextFormControlElement.cpp:

(WebCore::HTMLTextFormControlElement::indexForVisiblePosition): Use indexForPosition.
(WebCore::HTMLTextFormControlElement::computeSelectionStart): Ditto.
(WebCore::HTMLTextFormControlElement::computeSelectionEnd): Dotto.
(WebCore::finishText): Cleanup. Use newlineCharacter instead of hard-coding '\n'.
(WebCore::HTMLTextFormControlElement::indexForPosition): Added. See above for the description.

• html/HTMLTextFormControlElement.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• html/HTMLTextFormControlElement.cpp (modified) (5 diffs)
• html/HTMLTextFormControlElement.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-02-15  Ryosuke Niwa  <rniwa@webkit.org>
+
+        computeSelectionStart and computeSelectionEnd shouldn't trigger synchronous layout
+        https://bugs.webkit.org/show_bug.cgi?id=128806
+
+        Reviewed by Darin Adler.
+
+        Added indexForPosition to HTMLTextFormControlElement. Like r163825, this patch traverses the DOM tree
+        instead of the render tree to compute the index for a given position.
+
+        We traverse the DOM Tree backwards starting at the specified Position all the way back to the beginning
+        of the inner text element. The index is computed as the number of characters we encountered during
+        this backwards DOM traversal.
+
+        It's worth noting that passedPosition.computeNodeBeforePosition() returns and only returns 0 when the
+        position is before the first node of its parent or inside a text node. In such cases, we call
+        passedPosition.containerNode() to find the parent or the text node.
+
+        * html/HTMLTextFormControlElement.cpp:
+        (WebCore::HTMLTextFormControlElement::indexForVisiblePosition): Use indexForPosition.
+        (WebCore::HTMLTextFormControlElement::computeSelectionStart): Ditto.
+        (WebCore::HTMLTextFormControlElement::computeSelectionEnd): Dotto.
+        (WebCore::finishText): Cleanup. Use newlineCharacter instead of hard-coding '\n'.
+        (WebCore::HTMLTextFormControlElement::indexForPosition): Added. See above for the description.
+        * html/HTMLTextFormControlElement.h:
+
 2014-02-15  Brent Fulgham  <bfulgham@apple.com>
 

trunk/Source/WebCore/html/HTMLTextFormControlElement.cpp

@@ -331 +331 @@
     if (!innerText || !innerText->contains(position.deepEquivalent().anchorNode()))
         return 0;
-    bool forSelectionPreservation = false;
-    unsigned index = WebCore::indexForVisiblePosition(innerTextElement(), position, forSelectionPreservation);
+    unsigned index = indexForPosition(position.deepEquivalent());
     ASSERT(VisiblePosition(positionForIndex(innerTextElement(), index)) == position);
     return index;
@@ -361 +360 @@
         return 0;
 
-    return indexForVisiblePosition(frame->selection().selection().start());
+    return indexForPosition(frame->selection().selection().start());
 }
 
@@ -380 +379 @@
         return 0;
 
-    return indexForVisiblePosition(frame->selection().selection().end());
+    return indexForPosition(frame->selection().selection().end());
 }
 
@@ -538 +537 @@
     // Remove one trailing newline; there's always one that's collapsed out by rendering.
     size_t size = result.length();
-    if (size && result[size - 1] == '\n')
+    if (size && result[size - 1] == newlineCharacter)
         result.resize(--size);
     return result.toString();
@@ -578 +577 @@
     }
     return lastPositionInNode(innerText);
+}
+
+unsigned HTMLTextFormControlElement::indexForPosition(const Position& passedPosition) const
+{
+    TextControlInnerTextElement* innerText = innerTextElement();
+    if (!innerText || !innerText->contains(passedPosition.anchorNode()) || passedPosition.isNull())
+        return 0;
+
+    if (positionBeforeNode(innerText) == passedPosition)
+        return 0;
+
+    unsigned index = 0;
+    Node* startNode = passedPosition.computeNodeBeforePosition();
+    if (!startNode)
+        startNode = passedPosition.containerNode();
+    ASSERT(startNode);
+    ASSERT(innerText->contains(startNode));
+
+    for (Node* node = startNode; node; node = NodeTraversal::previous(node, innerText)) {
+        if (node->isTextNode()) {
+            unsigned length = toText(*node).length();
+            if (node == passedPosition.containerNode())
+                index += std::min<unsigned>(length, passedPosition.offsetInContainerNode());
+            else
+                index += length;
+        } else if (node->hasTagName(brTag))
+            index++;
+    }
+
+    unsigned length = innerTextValue().length();
+    index = std::min(index, length); // FIXME: We shouldn't have to call innerTextValue() just to ignore the last LF. See finishText.
+#ifndef ASSERT_DISABLED
+    VisiblePosition visiblePosition = passedPosition;
+    unsigned indexComputedByVisiblePosition = 0;
+    if (visiblePosition.isNotNull())
+        indexComputedByVisiblePosition = WebCore::indexForVisiblePosition(innerText, visiblePosition, false /* forSelectionPreservation */);
+    ASSERT(index == indexComputedByVisiblePosition);
+#endif
+    return index;
 }
 

trunk/Source/WebCore/html/HTMLTextFormControlElement.h

@@ -125 +125 @@
     virtual bool childShouldCreateRenderer(const Node&) const override;
 
+    unsigned indexForPosition(const Position&) const;
+
     // Returns true if user-editable value is empty. Used to check placeholder visibility.
     virtual bool isEmptyValue() const = 0;