Title: [163825] trunk/Source/WebCore
Revision
163825
Author
rn...@webkit.org
Date
2014-02-10 15:05:19 -0800 (Mon, 10 Feb 2014)

Log Message

HTMLTextFormControlElement::setSelectionRange shouldn't use VisiblePosition
https://bugs.webkit.org/show_bug.cgi?id=128478

Reviewed by Darin Adler.

Added positionForIndex to compute Position given a selection index. This function doesn't
synchronously trigger like visiblePositionForIndex.

Also added assertions in visiblePositionForIndex and visiblePositionForIndex to make sure
they are inverse of one another.

* html/HTMLTextFormControlElement.cpp:
(WebCore::HTMLTextFormControlElement::setSelectionRange): Use positionForIndex. Also removed
the now tautological assertions since we would never create a position outside the inner text
element.

(WebCore::HTMLTextFormControlElement::indexForVisiblePosition): Fixed the bug where this
function could return a selection index beyond innerTextElement in some types of input
elements such as search fields. fast/forms/search-disabled-readonly.html hits the newly
added assertion without this change. Note we can't use visiblePositionForIndex here as that
would result in a mutual recursion with the assertion in visiblePositionForIndex.

(WebCore::HTMLTextFormControlElement::visiblePositionForIndex): Added an assertion.

(WebCore::positionForIndex): Added. It's here with prototype at the beginning of the file
so that it's right next to HTMLTextFormControlElement::innerText() where we do a similar
DOM traversal to obtain the inner text value.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (163824 => 163825)


--- trunk/Source/WebCore/ChangeLog	2014-02-10 22:58:29 UTC (rev 163824)
+++ trunk/Source/WebCore/ChangeLog	2014-02-10 23:05:19 UTC (rev 163825)
@@ -1,3 +1,33 @@
+2014-02-10  Ryosuke Niwa  <rn...@webkit.org>
+
+        HTMLTextFormControlElement::setSelectionRange shouldn't use VisiblePosition
+        https://bugs.webkit.org/show_bug.cgi?id=128478
+
+        Reviewed by Darin Adler.
+
+        Added positionForIndex to compute Position given a selection index. This function doesn't
+        synchronously trigger like visiblePositionForIndex.
+
+        Also added assertions in visiblePositionForIndex and visiblePositionForIndex to make sure
+        they are inverse of one another.
+
+        * html/HTMLTextFormControlElement.cpp:
+        (WebCore::HTMLTextFormControlElement::setSelectionRange): Use positionForIndex. Also removed
+        the now tautological assertions since we would never create a position outside the inner text
+        element.
+
+        (WebCore::HTMLTextFormControlElement::indexForVisiblePosition): Fixed the bug where this
+        function could return a selection index beyond innerTextElement in some types of input
+        elements such as search fields. fast/forms/search-disabled-readonly.html hits the newly
+        added assertion without this change. Note we can't use visiblePositionForIndex here as that
+        would result in a mutual recursion with the assertion in visiblePositionForIndex.
+
+        (WebCore::HTMLTextFormControlElement::visiblePositionForIndex): Added an assertion.
+
+        (WebCore::positionForIndex): Added. It's here with prototype at the beginning of the file
+        so that it's right next to HTMLTextFormControlElement::innerText() where we do a similar
+        DOM traversal to obtain the inner text value.
+
 2014-02-07  Jeffrey Pfau  <jp...@apple.com>
 
         Disable access to application cache when in private browsing

Modified: trunk/Source/WebCore/html/HTMLTextFormControlElement.cpp (163824 => 163825)


--- trunk/Source/WebCore/html/HTMLTextFormControlElement.cpp	2014-02-10 22:58:29 UTC (rev 163824)
+++ trunk/Source/WebCore/html/HTMLTextFormControlElement.cpp	2014-02-10 23:05:19 UTC (rev 163825)
@@ -52,6 +52,8 @@
 
 using namespace HTMLNames;
 
+static Position positionForIndex(TextControlInnerTextElement*, unsigned);
+
 HTMLTextFormControlElement::HTMLTextFormControlElement(const QualifiedName& tagName, Document& document, HTMLFormElement* form)
     : HTMLFormControlElementWithState(tagName, document, form)
     , m_lastChangeWasUserEdit(false)
@@ -296,25 +298,18 @@
     end = std::max(end, 0);
     start = std::min(std::max(start, 0), end);
 
-    if (!hasVisibleTextArea(*renderer(), innerTextElement())) {
+    TextControlInnerTextElement* innerText = innerTextElement();
+    if (!hasVisibleTextArea(*renderer(), innerText)) {
         cacheSelection(start, end, direction);
         return;
     }
-    VisiblePosition startPosition = visiblePositionForIndex(start);
-    VisiblePosition endPosition;
+    Position startPosition = positionForIndex(innerText, start);
+    Position endPosition;
     if (start == end)
         endPosition = startPosition;
     else
-        endPosition = visiblePositionForIndex(end);
+        endPosition = positionForIndex(innerText, end);
 
-#if !PLATFORM(IOS)
-    // startPosition and endPosition can be null position for example when
-    // "-webkit-user-select: none" style attribute is specified.
-    if (startPosition.isNotNull() && endPosition.isNotNull()) {
-        ASSERT(startPosition.deepEquivalent().deprecatedNode()->shadowHost() == this
-            && endPosition.deepEquivalent().deprecatedNode()->shadowHost() == this);
-    }
-#endif
     VisibleSelection newSelection;
     if (direction == SelectionHasBackwardDirection)
         newSelection = VisibleSelection(endPosition, startPosition);
@@ -328,15 +323,20 @@
 
 int HTMLTextFormControlElement::indexForVisiblePosition(const VisiblePosition& pos) const
 {
-    if (enclosingTextFormControl(pos.deepEquivalent()) != this)
+    TextControlInnerTextElement* innerText = innerTextElement();
+    if (!innerText || !innerText->contains(pos.deepEquivalent().anchorNode()))
         return 0;
     bool forSelectionPreservation = false;
-    return WebCore::indexForVisiblePosition(innerTextElement(), pos, forSelectionPreservation);
+    unsigned index = WebCore::indexForVisiblePosition(innerTextElement(), pos, forSelectionPreservation);
+    ASSERT(VisiblePosition(positionForIndex(innerTextElement(), index)) == pos);
+    return index;
 }
 
 VisiblePosition HTMLTextFormControlElement::visiblePositionForIndex(int index) const
 {
-    return visiblePositionForIndexUsingCharacterIterator(innerTextElement(), index);
+    VisiblePosition position = positionForIndex(innerTextElement(), index);
+    ASSERT(indexForVisiblePosition(position) == index);
+    return position;
 }
 
 int HTMLTextFormControlElement::selectionStart() const
@@ -557,6 +557,24 @@
     return finishText(result);
 }
 
+static Position positionForIndex(TextControlInnerTextElement* innerText, unsigned index)
+{
+    unsigned remainingCharactersToMoveForward = index;
+    for (Node* node = innerText; node; node = NodeTraversal::next(node, innerText)) {
+        if (node->hasTagName(brTag)) {
+            if (!remainingCharactersToMoveForward)
+                return positionBeforeNode(node);
+            remainingCharactersToMoveForward--;
+        } else if (node->isTextNode()) {
+            Text* text = toText(node);
+            if (remainingCharactersToMoveForward < text->length())
+                return Position(text, remainingCharactersToMoveForward);
+            remainingCharactersToMoveForward -= text->length();
+        }
+    }
+    return lastPositionInNode(innerText);
+}
+
 #if PLATFORM(IOS)
 void HTMLTextFormControlElement::hidePlaceholder()
 {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to