Unduplicate the code to convert between VisiblePosition and index

https://bugs.webkit.org/show_bug.cgi?id=120506

Reviewed by Darin Adler.

Encapsulate the conversion between VisiblePosition and index into indexForVisiblePosition
and visiblePositionForIndexUsingCharacterIterator. It's unfortunate that these two functions
are different from the two other existing functions of similar names but we've at least
confined problems into a single cpp file now.

* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::visiblePositionForIndex):
(WebCore::AccessibilityRenderObject::indexForVisiblePosition):
* editing/Editor.cpp:
(WebCore::findFirstMarkable):
* editing/htmlediting.cpp:
(WebCore::indexForVisiblePosition):
(WebCore::visiblePositionForIndexUsingCharacterIterator):
* editing/htmlediting.h:
* html/HTMLTextFormControlElement.cpp:
(WebCore::hasVisibleTextArea):
(WebCore::HTMLTextFormControlElement::setSelectionRange):
(WebCore::HTMLTextFormControlElement::indexForVisiblePosition):
(WebCore::HTMLTextFormControlElement::visiblePositionForIndex):
* html/HTMLTextFormControlElement.h:
* rendering/RenderTextControl.cpp:
* rendering/RenderTextControl.h:

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 8d075552916e46ef71d010b3c734955c42b5a5b2..9a2f47419cf9cfcd3b9ec58349b89093c4e35f08 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,33 @@
+2013-08-29  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Unduplicate the code to convert between VisiblePosition and index
+        https://bugs.webkit.org/show_bug.cgi?id=120506
+
+        Reviewed by Darin Adler.
+
+        Encapsulate the conversion between VisiblePosition and index into indexForVisiblePosition
+        and visiblePositionForIndexUsingCharacterIterator. It's unfortunate that these two functions
+        are different from the two other existing functions of similar names but we've at least
+        confined problems into a single cpp file now.
+
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::AccessibilityRenderObject::visiblePositionForIndex):
+        (WebCore::AccessibilityRenderObject::indexForVisiblePosition):
+        * editing/Editor.cpp:
+        (WebCore::findFirstMarkable):
+        * editing/htmlediting.cpp:
+        (WebCore::indexForVisiblePosition):
+        (WebCore::visiblePositionForIndexUsingCharacterIterator):
+        * editing/htmlediting.h:
+        * html/HTMLTextFormControlElement.cpp:
+        (WebCore::hasVisibleTextArea):
+        (WebCore::HTMLTextFormControlElement::setSelectionRange):
+        (WebCore::HTMLTextFormControlElement::indexForVisiblePosition):
+        (WebCore::HTMLTextFormControlElement::visiblePositionForIndex):
+        * html/HTMLTextFormControlElement.h:
+        * rendering/RenderTextControl.cpp:
+        * rendering/RenderTextControl.h:
+
 2013-08-29  Ryosuke Niwa  <rniwa@webkit.org>
 
         Avoid Node references from AXObjectCache from leaking

diff --git a/Source/WebCore/accessibility/AccessibilityRenderObject.cpp b/Source/WebCore/accessibility/AccessibilityRenderObject.cpp
index aa52938811439f5ed01afe68cdc70d5b8ddabe0b..cb665cffe90c3410adacc43203504169ad792680 100644 (file)
--- a/Source/WebCore/accessibility/AccessibilityRenderObject.cpp
+++ b/Source/WebCore/accessibility/AccessibilityRenderObject.cpp
@@ -1816,9 +1816,9 @@ VisiblePosition AccessibilityRenderObject::visiblePositionForIndex(int index) co
 {
     if (!m_renderer)
         return VisiblePosition();
-    
+
     if (isNativeTextControl())
-        return toRenderTextControl(m_renderer)->visiblePositionForIndex(index);
+        return toRenderTextControl(m_renderer)->textFormControlElement()->visiblePositionForIndex(index);
 
     if (!allowsTextRanges() && !m_renderer->isText())
         return VisiblePosition();
@@ -1826,23 +1826,14 @@ VisiblePosition AccessibilityRenderObject::visiblePositionForIndex(int index) co
     Node* node = m_renderer->node();
     if (!node)
         return VisiblePosition();
-    
-    if (index <= 0)
-        return VisiblePosition(firstPositionInOrBeforeNode(node), DOWNSTREAM);
-    
-    RefPtr<Range> range = Range::create(&m_renderer->document());
-    range->selectNodeContents(node, IGNORE_EXCEPTION);
-    CharacterIterator it(range.get());
-    it.advance(index - 1);
-    return VisiblePosition(Position(it.range()->endContainer(), it.range()->endOffset(), Position::PositionIsOffsetInAnchor), UPSTREAM);
+
+    return visiblePositionForIndexUsingCharacterIterator(node, index);
 }
     
 int AccessibilityRenderObject::indexForVisiblePosition(const VisiblePosition& pos) const
 {
-    if (isNativeTextControl()) {
-        HTMLTextFormControlElement* textControl = toRenderTextControl(m_renderer)->textFormControlElement();
-        return textControl->indexForVisiblePosition(pos);
-    }
+    if (isNativeTextControl())
+        return toRenderTextControl(m_renderer)->textFormControlElement()->indexForVisiblePosition(pos);
 
     if (!isTextControl())
         return 0;
@@ -1850,22 +1841,20 @@ int AccessibilityRenderObject::indexForVisiblePosition(const VisiblePosition& po
     Node* node = m_renderer->node();
     if (!node)
         return 0;
-    
+
     Position indexPosition = pos.deepEquivalent();
     if (indexPosition.isNull() || highestEditableRoot(indexPosition, HasEditableAXRole) != node)
         return 0;
-    
-    RefPtr<Range> range = Range::create(&m_renderer->document());
-    range->setStart(node, 0, IGNORE_EXCEPTION);
-    range->setEnd(indexPosition, IGNORE_EXCEPTION);
 
 #if PLATFORM(GTK)
     // We need to consider replaced elements for GTK, as they will be
     // presented with the 'object replacement character' (0xFFFC).
-    return TextIterator::rangeLength(range.get(), true);
+    bool forSelectionPreservation = true;
 #else
-    return TextIterator::rangeLength(range.get());
+    bool forSelectionPreservation = false;
 #endif
+
+    return WebCore::indexForVisiblePosition(node, pos, forSelectionPreservation);
 }
 
 Element* AccessibilityRenderObject::rootEditableElementForPosition(const Position& position) const

diff --git a/Source/WebCore/editing/Editor.cpp b/Source/WebCore/editing/Editor.cpp
index ef7b1b340dc8605dbf9ba949986f0ddb5a46f768..ce0c5c5248fefef2d3b18db30f3645feb63b0387 100644 (file)
--- a/Source/WebCore/editing/Editor.cpp
+++ b/Source/WebCore/editing/Editor.cpp
@@ -3049,8 +3049,8 @@ static Node* findFirstMarkable(Node* node)
             return 0;
         if (node->renderer()->isText())
             return node;
-        if (node->renderer()->isTextControl())
-            node = toRenderTextControl(node->renderer())->visiblePositionForIndex(1).deepEquivalent().deprecatedNode();
+        if (isHTMLTextFormControlElement(node))
+            node = toHTMLTextFormControlElement(node)->visiblePositionForIndex(1).deepEquivalent().deprecatedNode();
         else if (node->firstChild())
             node = node->firstChild();
         else

diff --git a/Source/WebCore/editing/htmlediting.cpp b/Source/WebCore/editing/htmlediting.cpp
index c725a477cefb65edbb878853afae0d516f48232d..ae2ad9d5577abba8f1efea55e7a2732e6eef6539 100644 (file)
--- a/Source/WebCore/editing/htmlediting.cpp
+++ b/Source/WebCore/editing/htmlediting.cpp
@@ -1159,6 +1159,14 @@ int indexForVisiblePosition(const VisiblePosition& visiblePosition, RefPtr<Conta
     return TextIterator::rangeLength(range.get(), true);
 }
 
+// FIXME: Merge these two functions.
+int indexForVisiblePosition(Node* node, const VisiblePosition& visiblePosition, bool forSelectionPreservation)
+{
+    ASSERT(node);
+    RefPtr<Range> range = Range::create(node->document(), firstPositionInNode(node), visiblePosition.deepEquivalent().parentAnchoredEquivalent());
+    return TextIterator::rangeLength(range.get(), forSelectionPreservation);
+}
+
 VisiblePosition visiblePositionForIndex(int index, ContainerNode* scope)
 {
     RefPtr<Range> range = TextIterator::rangeFromLocationAndLength(scope, index, 0, true);
@@ -1169,6 +1177,20 @@ VisiblePosition visiblePositionForIndex(int index, ContainerNode* scope)
     return VisiblePosition(range->startPosition());
 }
 
+VisiblePosition visiblePositionForIndexUsingCharacterIterator(Node* node, int index)
+{
+    ASSERT(node);
+    if (index <= 0)
+        return VisiblePosition(firstPositionInOrBeforeNode(node), DOWNSTREAM);
+
+    RefPtr<Range> range = Range::create(node->document());
+    range->selectNodeContents(node, IGNORE_EXCEPTION);
+    CharacterIterator it(range.get());
+    it.advance(index - 1);
+
+    return VisiblePosition(Position(it.range()->endContainer(), it.range()->endOffset(), Position::PositionIsOffsetInAnchor), UPSTREAM);
+}
+
 // Determines whether two positions are visibly next to each other (first then second)
 // while ignoring whitespaces and unrendered nodes
 bool isVisiblyAdjacent(const Position& first, const Position& second)

diff --git a/Source/WebCore/editing/htmlediting.h b/Source/WebCore/editing/htmlediting.h
index 600fffc8f3342b1439967409eda755a9da8fac82..85c020ead87f9a597d5298d33fa53c80d849226b 100644 (file)
--- a/Source/WebCore/editing/htmlediting.h
+++ b/Source/WebCore/editing/htmlediting.h
@@ -189,7 +189,9 @@ bool lineBreakExistsAtVisiblePosition(const VisiblePosition&);
 int comparePositions(const VisiblePosition&, const VisiblePosition&);
 
 int indexForVisiblePosition(const VisiblePosition&, RefPtr<ContainerNode>& scope);
+int indexForVisiblePosition(Node*, const VisiblePosition&, bool forSelectionPreservation);
 VisiblePosition visiblePositionForIndex(int index, ContainerNode* scope);
+VisiblePosition visiblePositionForIndexUsingCharacterIterator(Node*, int index); // FIXME: Why do we need this version?
 
 // -------------------------------------------------------------------------
 // Range

diff --git a/Source/WebCore/html/HTMLTextFormControlElement.cpp b/Source/WebCore/html/HTMLTextFormControlElement.cpp
index b7cc2799764e29495a44f705246a7b752a01ec67..d42100abf197c52ff72ad4b29e45d3ca513c7093 100644 (file)
--- a/Source/WebCore/html/HTMLTextFormControlElement.cpp
+++ b/Source/WebCore/html/HTMLTextFormControlElement.cpp
@@ -39,13 +39,12 @@
 #include "HTMLInputElement.h"
 #include "HTMLNames.h"
 #include "NodeTraversal.h"
-#include "RenderBox.h"
-#include "RenderTextControl.h"
+#include "RenderBlock.h"
 #include "RenderTheme.h"
 #include "ScriptEventListener.h"
 #include "ShadowRoot.h"
 #include "Text.h"
-#include "TextIterator.h"
+#include "htmlediting.h"
 #include <wtf/text/StringBuilder.h>
 
 namespace WebCore {
@@ -223,13 +222,12 @@ void HTMLTextFormControlElement::dispatchFormControlChangeEvent()
     setChangedSinceLastFormControlChangeEvent(false);
 }
 
-static inline bool hasVisibleTextArea(RenderTextControl* textControl, HTMLElement* innerText)
+static inline bool hasVisibleTextArea(RenderObject* textControl, HTMLElement* innerText)
 {
     ASSERT(textControl);
     return textControl->style()->visibility() != HIDDEN && innerText && innerText->renderer() && innerText->renderBox()->height();
 }
 
-
 void HTMLTextFormControlElement::setRangeText(const String& replacement, ExceptionCode& ec)
 {
     setRangeText(replacement, selectionStart(), selectionEnd(), String(), ec);
@@ -310,17 +308,16 @@ void HTMLTextFormControlElement::setSelectionRange(int start, int end, TextField
     end = max(end, 0);
     start = min(max(start, 0), end);
 
-    RenderTextControl* control = toRenderTextControl(renderer());
-    if (!hasVisibleTextArea(control, innerTextElement())) {
+    if (!hasVisibleTextArea(renderer(), innerTextElement())) {
         cacheSelection(start, end, direction);
         return;
     }
-    VisiblePosition startPosition = control->visiblePositionForIndex(start);
+    VisiblePosition startPosition = visiblePositionForIndex(start);
     VisiblePosition endPosition;
     if (start == end)
         endPosition = startPosition;
     else
-        endPosition = control->visiblePositionForIndex(end);
+        endPosition = visiblePositionForIndex(end);
 
     // startPosition and endPosition can be null position for example when
     // "-webkit-user-select: none" style attribute is specified.
@@ -341,13 +338,15 @@ void HTMLTextFormControlElement::setSelectionRange(int start, int end, TextField
 
 int HTMLTextFormControlElement::indexForVisiblePosition(const VisiblePosition& pos) const
 {
-    Position indexPosition = pos.deepEquivalent().parentAnchoredEquivalent();
-    if (enclosingTextFormControl(indexPosition) != this)
+    if (enclosingTextFormControl(pos.deepEquivalent()) != this)
         return 0;
-    RefPtr<Range> range = Range::create(indexPosition.document());
-    range->setStart(innerTextElement(), 0, ASSERT_NO_EXCEPTION);
-    range->setEnd(indexPosition.containerNode(), indexPosition.offsetInContainerNode(), ASSERT_NO_EXCEPTION);
-    return TextIterator::rangeLength(range.get());
+    bool forSelectionPreservation = false;
+    return WebCore::indexForVisiblePosition(innerTextElement(), pos, forSelectionPreservation);
+}
+
+VisiblePosition HTMLTextFormControlElement::visiblePositionForIndex(int index) const
+{
+    return visiblePositionForIndexUsingCharacterIterator(innerTextElement(), index);
 }
 
 int HTMLTextFormControlElement::selectionStart() const

diff --git a/Source/WebCore/html/HTMLTextFormControlElement.h b/Source/WebCore/html/HTMLTextFormControlElement.h
index 0cc3b0886fd2394e939e9057ce7a6092992ed7f8..3a4fe97d886bfc24cf0681fa9a54d4b9a64fbede 100644 (file)
--- a/Source/WebCore/html/HTMLTextFormControlElement.h
+++ b/Source/WebCore/html/HTMLTextFormControlElement.h
@@ -57,6 +57,7 @@ public:
     static void fixPlaceholderRenderer(HTMLElement* placeholder, HTMLElement* siblingElement);
 
     int indexForVisiblePosition(const VisiblePosition&) const;
+    VisiblePosition visiblePositionForIndex(int index) const;
     int selectionStart() const;
     int selectionEnd() const;
     const AtomicString& selectionDirection() const;

diff --git a/Source/WebCore/rendering/RenderTextControl.cpp b/Source/WebCore/rendering/RenderTextControl.cpp
index d4b785ea8ce4fc24b02b68d5becd7a1d9085adb9..10779a9c932b3af3a72164632b8677072f4bd36a 100644 (file)
--- a/Source/WebCore/rendering/RenderTextControl.cpp
+++ b/Source/WebCore/rendering/RenderTextControl.cpp
@@ -126,17 +126,6 @@ void RenderTextControl::updateFromElement()
         updateUserModifyProperty(node(), innerText->renderer()->style());
 }
 
-VisiblePosition RenderTextControl::visiblePositionForIndex(int index) const
-{
-    if (index <= 0)
-        return VisiblePosition(firstPositionInNode(innerTextElement()), DOWNSTREAM);
-    RefPtr<Range> range = Range::create(&document());
-    range->selectNodeContents(innerTextElement(), ASSERT_NO_EXCEPTION);
-    CharacterIterator it(range.get());
-    it.advance(index - 1);
-    return VisiblePosition(it.range()->endPosition(), UPSTREAM);
-}
-
 int RenderTextControl::scrollbarThickness() const
 {
     // FIXME: We should get the size of the scrollbar from the RenderTheme instead.

diff --git a/Source/WebCore/rendering/RenderTextControl.h b/Source/WebCore/rendering/RenderTextControl.h
index 3bc869925d9854ebc79959646788c2a718dc63b1..30712612299e504f2a0364cad8a35a2304ed3bd7 100644 (file)
--- a/Source/WebCore/rendering/RenderTextControl.h
+++ b/Source/WebCore/rendering/RenderTextControl.h
@@ -36,8 +36,6 @@ public:
     HTMLTextFormControlElement* textFormControlElement() const;
     virtual PassRefPtr<RenderStyle> createInnerTextStyle(const RenderStyle* startStyle) const = 0;
 
-    VisiblePosition visiblePositionForIndex(int index) const;
-
 protected:
     RenderTextControl(Element*);