HTMLTextFormControlElement::setSelectionRange shouldn't use VisiblePosition
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Feb 2014 23:05:19 +0000 (23:05 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Feb 2014 23:05:19 +0000 (23:05 +0000)
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.

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

Source/WebCore/ChangeLog
Source/WebCore/html/HTMLTextFormControlElement.cpp

index 614410107dcf118702c8eef5a5e974bf78ad93b5..6d8d5dea1b3a2db426357c7cfe4aa025ed1f4b52 100644 (file)
@@ -1,3 +1,33 @@
+2014-02-10  Ryosuke Niwa  <rniwa@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  <jpfau@apple.com>
 
         Disable access to application cache when in private browsing
index 06b52268422fe799326dfc8091a29c087a490779..1c29a646e982ac286fe9efc92776d7ff3a86e7a3 100644 (file)
@@ -52,6 +52,8 @@ namespace WebCore {
 
 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 @@ void HTMLTextFormControlElement::setSelectionRange(int start, int end, TextField
     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);
-
-#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
+        endPosition = positionForIndex(innerText, end);
+
     VisibleSelection newSelection;
     if (direction == SelectionHasBackwardDirection)
         newSelection = VisibleSelection(endPosition, startPosition);
@@ -328,15 +323,20 @@ void HTMLTextFormControlElement::setSelectionRange(int start, int end, TextField
 
 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 @@ String HTMLTextFormControlElement::innerTextValue() const
     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()
 {