Enable the offset assertion in HTMLTextFormControlElement::indexForPosition
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 Jan 2020 05:57:39 +0000 (05:57 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 Jan 2020 05:57:39 +0000 (05:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=205706

Reviewed by Darin Adler.

Source/WebCore:

This patch fixes the erroneously disabled debug assertion in HTMLTextFormControlElement::indexForPosition.

It also fixes the bug that it was asserting even when VisiblePosition was null, and computed a wrong offset
when the entire input element is not visible (e.g. becaue height is 0px).

TextIterator::rangeLength and TextIterator::rangeFromLocationAndLength now takes an OptionSet of
newly added enum class TextIteratorLengthOption instead of a boolean indicating whether a space should be
generated for a replaced element. Most code changes are due to this refactoring.

No new tests since existing tests exercise this code.

* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::rangeMatchesTextNearRange):
* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::indexForVisiblePosition const):
* accessibility/atk/WebKitAccessibleInterfaceText.cpp:
(getSelectionOffsetsForObject):
* accessibility/atk/WebKitAccessibleUtil.cpp:
(objectFocusedAndCaretOffsetUnignored):
* editing/ApplyStyleCommand.cpp:
(WebCore::ApplyStyleCommand::applyBlockStyle):
* editing/CompositeEditCommand.cpp:
(WebCore::CompositeEditCommand::moveParagraphs):
* editing/Editing.cpp:
(WebCore::indexForVisiblePosition):
(WebCore::visiblePositionForIndex):
* editing/Editing.h:
(WebCore::indexForVisiblePosition):
* editing/TextIterator.cpp:
(WebCore::behaviorFromLegnthOptions): Added.
(WebCore::TextIterator::rangeLength):
(WebCore::TextIterator::rangeFromLocationAndLength):
* editing/TextIterator.h:
(WebCore::TextIterator::rangeLength):
(WebCore::TextIterator::rangeFromLocationAndLength):
* editing/TextIteratorBehavior.h:
* editing/ios/DictationCommandIOS.cpp:
(WebCore::DictationCommandIOS::doApply):
* html/HTMLTextFormControlElement.cpp:
(WebCore::HTMLTextFormControlElement::indexForPosition const): Enabled the assertion when VisiblePosition
is not null, and fixed the bug that the offset computed from VisiblePosition were always 0 when the input
element is not visible (e.g. has 0px size or has visibility: hidden).
* page/EventHandler.cpp:
(WebCore::textDistance):

Source/WebKit:

* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::rangeNearPositionMatchesText):

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

18 files changed:
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AXObjectCache.cpp
Source/WebCore/accessibility/AccessibilityRenderObject.cpp
Source/WebCore/accessibility/atk/WebKitAccessibleHyperlink.cpp
Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp
Source/WebCore/accessibility/atk/WebKitAccessibleUtil.cpp
Source/WebCore/editing/ApplyStyleCommand.cpp
Source/WebCore/editing/CompositeEditCommand.cpp
Source/WebCore/editing/Editing.cpp
Source/WebCore/editing/Editing.h
Source/WebCore/editing/TextIterator.cpp
Source/WebCore/editing/TextIterator.h
Source/WebCore/editing/TextIteratorBehavior.h
Source/WebCore/editing/ios/DictationCommandIOS.cpp
Source/WebCore/html/HTMLTextFormControlElement.cpp
Source/WebCore/page/EventHandler.cpp
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

index 79abe30..7a48247 100644 (file)
@@ -1,3 +1,55 @@
+2020-01-14  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Enable the offset assertion in HTMLTextFormControlElement::indexForPosition
+        https://bugs.webkit.org/show_bug.cgi?id=205706
+
+        Reviewed by Darin Adler.
+
+        This patch fixes the erroneously disabled debug assertion in HTMLTextFormControlElement::indexForPosition.
+
+        It also fixes the bug that it was asserting even when VisiblePosition was null, and computed a wrong offset
+        when the entire input element is not visible (e.g. becaue height is 0px).
+
+        TextIterator::rangeLength and TextIterator::rangeFromLocationAndLength now takes an OptionSet of
+        newly added enum class TextIteratorLengthOption instead of a boolean indicating whether a space should be
+        generated for a replaced element. Most code changes are due to this refactoring.
+
+        No new tests since existing tests exercise this code.
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::rangeMatchesTextNearRange):
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::AccessibilityRenderObject::indexForVisiblePosition const):
+        * accessibility/atk/WebKitAccessibleInterfaceText.cpp:
+        (getSelectionOffsetsForObject):
+        * accessibility/atk/WebKitAccessibleUtil.cpp:
+        (objectFocusedAndCaretOffsetUnignored):
+        * editing/ApplyStyleCommand.cpp:
+        (WebCore::ApplyStyleCommand::applyBlockStyle):
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::moveParagraphs):
+        * editing/Editing.cpp:
+        (WebCore::indexForVisiblePosition):
+        (WebCore::visiblePositionForIndex):
+        * editing/Editing.h:
+        (WebCore::indexForVisiblePosition):
+        * editing/TextIterator.cpp:
+        (WebCore::behaviorFromLegnthOptions): Added.
+        (WebCore::TextIterator::rangeLength):
+        (WebCore::TextIterator::rangeFromLocationAndLength):
+        * editing/TextIterator.h:
+        (WebCore::TextIterator::rangeLength):
+        (WebCore::TextIterator::rangeFromLocationAndLength):
+        * editing/TextIteratorBehavior.h:
+        * editing/ios/DictationCommandIOS.cpp:
+        (WebCore::DictationCommandIOS::doApply):
+        * html/HTMLTextFormControlElement.cpp:
+        (WebCore::HTMLTextFormControlElement::indexForPosition const): Enabled the assertion when VisiblePosition
+        is not null, and fixed the bug that the offset computed from VisiblePosition were always 0 when the input
+        element is not visible (e.g. has 0px size or has visibility: hidden).
+        * page/EventHandler.cpp:
+        (WebCore::textDistance):
+
 2020-01-14  Chris Dumez  <cdumez@apple.com>
 
         document.cookie should not do a sync IPC to the network process for iframes that do not have storage access
index 1e6d610..dc6c04a 100644 (file)
@@ -1933,7 +1933,7 @@ RefPtr<Range> AXObjectCache::rangeMatchesTextNearRange(RefPtr<Range> originalRan
         return nullptr;
     
     auto range = Range::create(m_document, startPosition, originalRange->startPosition());
-    unsigned targetOffset = TextIterator::rangeLength(range.ptr(), true);
+    unsigned targetOffset = TextIterator::rangeLength(range.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     return findClosestPlainText(searchRange.get(), matchText, { }, targetOffset);
 }
 
index a87237c..514788a 100644 (file)
@@ -2032,12 +2032,10 @@ int AccessibilityRenderObject::indexForVisiblePosition(const VisiblePosition& po
 #if USE(ATK)
     // We need to consider replaced elements for GTK, as they will be
     // presented with the 'object replacement character' (0xFFFC).
-    bool forSelectionPreservation = true;
+    return WebCore::indexForVisiblePosition(*node, position, { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
 #else
-    bool forSelectionPreservation = false;
+    return WebCore::indexForVisiblePosition(*node, position);
 #endif
-
-    return WebCore::indexForVisiblePosition(*node, position, forSelectionPreservation);
 }
 
 Element* AccessibilityRenderObject::rootEditableElementForPosition(const Position& position) const
index a461c1f..fd13fc4 100644 (file)
@@ -154,7 +154,7 @@ static AtkObject* webkitAccessibleHyperlinkGetObject(AtkHyperlink* link, gint in
 static gint rangeLengthForObject(AccessibilityObject& obj, Range* range)
 {
     // This is going to be the actual length in most of the cases
-    int baseLength = TextIterator::rangeLength(range, true);
+    int baseLength = TextIterator::rangeLength(range, { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
 
     // Check whether the current hyperlink belongs to a list item.
     // If so, we need to consider the length of the item's marker
index b2715c7..08fc78b 100644 (file)
@@ -410,9 +410,9 @@ static void getSelectionOffsetsForObject(AccessibilityObject* coreObject, Visibl
 
     // Set values for start offsets and calculate initial range length.
     // These values might be adjusted later to cover special cases.
-    startOffset = webCoreOffsetToAtkOffset(coreObject, TextIterator::rangeLength(rangeInParent.ptr(), true));
+    startOffset = webCoreOffsetToAtkOffset(coreObject, TextIterator::rangeLength(rangeInParent.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements }));
     auto nodeRange = Range::create(node->document(), nodeRangeStart, nodeRangeEnd);
-    int rangeLength = TextIterator::rangeLength(nodeRange.ptr(), true);
+    int rangeLength = TextIterator::rangeLength(nodeRange.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
 
     // Special cases that are only relevant when working with *_END boundaries.
     if (selection.affinity() == UPSTREAM) {
index 56392c0..509dee5 100644 (file)
@@ -247,10 +247,10 @@ AXCoreObject* objectFocusedAndCaretOffsetUnignored(AXCoreObject* referenceObject
         offset = 0;
     else if (!isStartOfLine(endPosition)) {
         RefPtr<Range> range = makeRange(startPosition, endPosition.previous());
-        offset = TextIterator::rangeLength(range.get(), true) + 1;
+        offset = TextIterator::rangeLength(range.get(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements }) + 1;
     } else {
         RefPtr<Range> range = makeRange(startPosition, endPosition);
-        offset = TextIterator::rangeLength(range.get(), true);
+        offset = TextIterator::rangeLength(range.get(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     }
 
     return firstUnignoredParent;
index 52e6e4e..d80b021 100644 (file)
@@ -252,8 +252,8 @@ void ApplyStyleCommand::applyBlockStyle(EditingStyle& style)
 
     auto startRange = Range::create(document(), firstPositionInNode(scope), visibleStart.deepEquivalent().parentAnchoredEquivalent());
     auto endRange = Range::create(document(), firstPositionInNode(scope), visibleEnd.deepEquivalent().parentAnchoredEquivalent());
-    int startIndex = TextIterator::rangeLength(startRange.ptr(), true);
-    int endIndex = TextIterator::rangeLength(endRange.ptr(), true);
+    int startIndex = TextIterator::rangeLength(startRange.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
+    int endIndex = TextIterator::rangeLength(endRange.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
 
     VisiblePosition paragraphStart(startOfParagraph(visibleStart));
     VisiblePosition nextParagraphStart(endOfParagraph(paragraphStart).next());
@@ -285,8 +285,8 @@ void ApplyStyleCommand::applyBlockStyle(EditingStyle& style)
     }
     
     {
-        auto startRange = TextIterator::rangeFromLocationAndLength(scope, startIndex, 0, true);
-        auto endRange = TextIterator::rangeFromLocationAndLength(scope, endIndex, 0, true);
+        auto startRange = TextIterator::rangeFromLocationAndLength(scope, startIndex, 0, { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
+        auto endRange = TextIterator::rangeFromLocationAndLength(scope, endIndex, 0, { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
         if (startRange && endRange)
             updateStartEnd(startRange->startPosition(), endRange->startPosition());
     }
index 5dab7ba..ffffff1 100644 (file)
@@ -1433,13 +1433,13 @@ void CompositeEditCommand::moveParagraphs(const VisiblePosition& startOfParagrap
             startIndex = 0;
             if (startInParagraph) {
                 auto startRange = Range::create(document(), startOfParagraphToMove.deepEquivalent().parentAnchoredEquivalent(), visibleStart.deepEquivalent().parentAnchoredEquivalent());
-                startIndex = TextIterator::rangeLength(startRange.ptr(), true);
+                startIndex = TextIterator::rangeLength(startRange.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
             }
 
             endIndex = 0;
             if (endInParagraph) {
                 auto endRange = Range::create(document(), startOfParagraphToMove.deepEquivalent().parentAnchoredEquivalent(), visibleEnd.deepEquivalent().parentAnchoredEquivalent());
-                endIndex = TextIterator::rangeLength(endRange.ptr(), true);
+                endIndex = TextIterator::rangeLength(endRange.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
             }
         }
     }
@@ -1510,7 +1510,7 @@ void CompositeEditCommand::moveParagraphs(const VisiblePosition& startOfParagrap
         editableRoot = &document();
 
     auto startToDestinationRange = Range::create(document(), firstPositionInNode(editableRoot.get()), destination.deepEquivalent().parentAnchoredEquivalent());
-    destinationIndex = TextIterator::rangeLength(startToDestinationRange.ptr(), true);
+    destinationIndex = TextIterator::rangeLength(startToDestinationRange.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
 
     setEndingSelection(VisibleSelection(destination, originalIsDirectional));
     ASSERT(endingSelection().isCaretOrRange());
@@ -1532,8 +1532,10 @@ void CompositeEditCommand::moveParagraphs(const VisiblePosition& startOfParagrap
         // causes spaces to be collapsed during the move operation.  This results
         // in a call to rangeFromLocationAndLength with a location past the end
         // of the document (which will return null).
-        RefPtr<Range> start = TextIterator::rangeFromLocationAndLength(editableRoot.get(), destinationIndex + startIndex, 0, true);
-        RefPtr<Range> end = TextIterator::rangeFromLocationAndLength(editableRoot.get(), destinationIndex + endIndex, 0, true);
+        RefPtr<Range> start = TextIterator::rangeFromLocationAndLength(editableRoot.get(), destinationIndex + startIndex, 0,
+            { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
+        RefPtr<Range> end = TextIterator::rangeFromLocationAndLength(editableRoot.get(), destinationIndex + endIndex, 0,
+            { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
         if (start && end)
             setEndingSelection(VisibleSelection(start->startPosition(), end->startPosition(), DOWNSTREAM, originalIsDirectional));
     }
index fd5e595..0c2868d 100644 (file)
@@ -1084,14 +1084,14 @@ int indexForVisiblePosition(const VisiblePosition& visiblePosition, RefPtr<Conta
     }
 
     auto range = Range::create(document, firstPositionInNode(scope.get()), position.parentAnchoredEquivalent());
-    return TextIterator::rangeLength(range.ptr(), true);
+    return TextIterator::rangeLength(range.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
 }
 
 // FIXME: Merge this function with the one above.
-int indexForVisiblePosition(Node& node, const VisiblePosition& visiblePosition, bool forSelectionPreservation)
+int indexForVisiblePosition(Node& node, const VisiblePosition& visiblePosition, OptionSet<TextIteratorLengthOption> options)
 {
     auto range = Range::create(node.document(), firstPositionInNode(&node), visiblePosition.deepEquivalent().parentAnchoredEquivalent());
-    return TextIterator::rangeLength(range.ptr(), forSelectionPreservation);
+    return TextIterator::rangeLength(range.ptr(), options);
 }
 
 VisiblePosition visiblePositionForPositionWithOffset(const VisiblePosition& position, int offset)
@@ -1106,7 +1106,7 @@ VisiblePosition visiblePositionForPositionWithOffset(const VisiblePosition& posi
 
 VisiblePosition visiblePositionForIndex(int index, ContainerNode* scope)
 {
-    auto range = TextIterator::rangeFromLocationAndLength(scope, index, 0, true);
+    auto range = TextIterator::rangeFromLocationAndLength(scope, index, 0, { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     // Check for an invalid index. Certain editing operations invalidate indices because 
     // of problems with TextIteratorEmitsCharactersBetweenAllVisiblePositions.
     if (!range)
index 745b763..09589f3 100644 (file)
 #pragma once
 
 #include "Position.h"
+#include "TextIteratorBehavior.h"
 #include <wtf/Forward.h>
 #include <wtf/HashSet.h>
+#include <wtf/OptionSet.h>
 #include <wtf/unicode/CharacterNames.h>
 
 namespace WebCore {
@@ -149,7 +151,7 @@ bool lineBreakExistsAtVisiblePosition(const VisiblePosition&);
 WEBCORE_EXPORT int comparePositions(const VisiblePosition&, const VisiblePosition&);
 
 WEBCORE_EXPORT int indexForVisiblePosition(const VisiblePosition&, RefPtr<ContainerNode>& scope);
-int indexForVisiblePosition(Node&, const VisiblePosition&, bool forSelectionPreservation);
+int indexForVisiblePosition(Node&, const VisiblePosition&, OptionSet<TextIteratorLengthOption> = { });
 WEBCORE_EXPORT VisiblePosition visiblePositionForPositionWithOffset(const VisiblePosition&, int offset);
 WEBCORE_EXPORT VisiblePosition visiblePositionForIndex(int index, ContainerNode* scope);
 VisiblePosition visiblePositionForIndexUsingCharacterIterator(Node&, int index); // FIXME: Why do we need this version?
index 53c2d25..a111bdf 100644 (file)
@@ -2396,10 +2396,20 @@ size_t SearchBuffer::length() const
 
 // --------
 
-int TextIterator::rangeLength(const Range* range, bool forSelectionPreservation)
+static TextIteratorBehavior behaviorFromLegnthOptions(OptionSet<TextIteratorLengthOption> options)
+{
+    TextIteratorBehavior behavior = TextIteratorDefaultBehavior;
+    if (options.contains(TextIteratorLengthOption::GenerateSpacesForReplacedElements))
+        behavior |= TextIteratorEmitsCharactersBetweenAllVisiblePositions;
+    if (options.contains(TextIteratorLengthOption::IgnoreVisibility))
+        behavior |= TextIteratorIgnoresStyleVisibility;
+    return behavior;
+}
+
+int TextIterator::rangeLength(const Range* range, OptionSet<TextIteratorLengthOption> options)
 {
     unsigned length = 0;
-    for (TextIterator it(range, forSelectionPreservation ? TextIteratorEmitsCharactersBetweenAllVisiblePositions : TextIteratorDefaultBehavior); !it.atEnd(); it.advance())
+    for (TextIterator it(range, behaviorFromLegnthOptions(options)); !it.atEnd(); it.advance())
         length += it.text().length();
     return length;
 }
@@ -2418,7 +2428,7 @@ static inline bool isInsideReplacedElement(TextIterator& iterator)
     return node && isRendererReplacedElement(node->renderer());
 }
 
-RefPtr<Range> TextIterator::rangeFromLocationAndLength(ContainerNode* scope, int rangeLocation, int rangeLength, bool forSelectionPreservation)
+RefPtr<Range> TextIterator::rangeFromLocationAndLength(ContainerNode* scope, int rangeLocation, int rangeLength, OptionSet<TextIteratorLengthOption> options)
 {
     Ref<Range> resultRange = scope->document().createRange();
 
@@ -2428,7 +2438,7 @@ RefPtr<Range> TextIterator::rangeFromLocationAndLength(ContainerNode* scope, int
 
     Ref<Range> textRunRange = rangeOfContents(*scope);
 
-    TextIterator it(textRunRange.ptr(), forSelectionPreservation ? TextIteratorEmitsCharactersBetweenAllVisiblePositions : TextIteratorDefaultBehavior);
+    TextIterator it(textRunRange.ptr(), behaviorFromLegnthOptions(options));
     
     // FIXME: the atEnd() check shouldn't be necessary, workaround for <http://bugs.webkit.org/show_bug.cgi?id=6289>.
     if (!rangeLocation && !rangeLength && it.atEnd()) {
index 10304af..92aea7e 100644 (file)
@@ -31,6 +31,7 @@
 #include "LineLayoutTraversal.h"
 #include "Range.h"
 #include "TextIteratorBehavior.h"
+#include <wtf/OptionSet.h>
 #include <wtf/Vector.h>
 #include <wtf/text/StringView.h>
 
@@ -121,8 +122,8 @@ public:
     const TextIteratorCopyableText& copyableText() const { ASSERT(!atEnd()); return m_copyableText; }
     void appendTextToStringBuilder(StringBuilder& builder) const { copyableText().appendToStringBuilder(builder); }
 
-    WEBCORE_EXPORT static int rangeLength(const Range*, bool spacesForReplacedElements = false);
-    WEBCORE_EXPORT static RefPtr<Range> rangeFromLocationAndLength(ContainerNode* scope, int rangeLocation, int rangeLength, bool spacesForReplacedElements = false);
+    WEBCORE_EXPORT static int rangeLength(const Range*, OptionSet<TextIteratorLengthOption> = { });
+    WEBCORE_EXPORT static RefPtr<Range> rangeFromLocationAndLength(ContainerNode* scope, int rangeLocation, int rangeLength, OptionSet<TextIteratorLengthOption> = { });
     WEBCORE_EXPORT static bool getLocationAndLengthFromRange(Node* scope, const Range*, size_t& location, size_t& length);
     WEBCORE_EXPORT static Ref<Range> subrange(Range& entireRange, int characterOffset, int characterCount);
 
index c01f97f..a570ec0 100644 (file)
@@ -63,4 +63,9 @@ enum TextIteratorBehaviorFlag {
 
 typedef unsigned short TextIteratorBehavior;
 
+enum class TextIteratorLengthOption : uint8_t {
+    GenerateSpacesForReplacedElements = 1 << 0,
+    IgnoreVisibility = 1 << 1,
+};
+
 } // namespace WebCore
index 9c1ae88..c18edc0 100644 (file)
@@ -68,11 +68,12 @@ void DictationCommandIOS::doApply()
 
     // FIXME: Add the result marker using a Position cached before results are inserted, instead of relying on TextIterators.
     auto rangeToEnd = Range::create(document(), createLegacyEditingPosition((Node *)root, 0), afterResults.deepEquivalent());
-    int endIndex = TextIterator::rangeLength(rangeToEnd.ptr(), true);
+    int endIndex = TextIterator::rangeLength(rangeToEnd.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     int startIndex = endIndex - resultLength;
 
     if (startIndex >= 0) {
-        RefPtr<Range> resultRange = TextIterator::rangeFromLocationAndLength(document().documentElement(), startIndex, endIndex, true);
+        RefPtr<Range> resultRange = TextIterator::rangeFromLocationAndLength(document().documentElement(), startIndex, endIndex,
+            { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
         ASSERT(resultRange); // FIXME: What guarantees this?
         document().markers().addDictationResultMarker(*resultRange, m_metadata);
     }
index 3f3cfac..f5342f5 100644 (file)
@@ -643,16 +643,13 @@ unsigned HTMLTextFormControlElement::indexForPosition(const Position& passedPosi
 
     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.
-#if 0
-    // FIXME: This assertion code was never built, has bit rotted, and needs to be fixed before it can be enabled:
-    // https://bugs.webkit.org/show_bug.cgi?id=205706.
 #if ASSERT_ENABLED
     VisiblePosition visiblePosition = passedPosition;
-    unsigned indexComputedByVisiblePosition = 0;
-    if (visiblePosition.isNotNull())
-        indexComputedByVisiblePosition = WebCore::indexForVisiblePosition(innerText, visiblePosition, false /* forSelectionPreservation */);
-    ASSERT(index == indexComputedByVisiblePosition);
-#endif
+    if (visiblePosition.isNotNull()) {
+        unsigned indexComputedByVisiblePosition = WebCore::indexForVisiblePosition(*innerText, visiblePosition,
+            { TextIteratorLengthOption::GenerateSpacesForReplacedElements, TextIteratorLengthOption::IgnoreVisibility });
+        ASSERT(index == indexComputedByVisiblePosition);
+    }
 #endif
     return index;
 }
index 9a21e70..a752672 100644 (file)
@@ -657,7 +657,7 @@ bool EventHandler::handleMousePressEventTripleClick(const MouseEventWithHitTestR
 static int textDistance(const Position& start, const Position& end)
 {
     auto range = Range::create(start.anchorNode()->document(), start, end);
-    return TextIterator::rangeLength(range.ptr(), true);
+    return TextIterator::rangeLength(range.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
 }
 
 bool EventHandler::handleMousePressEventSingleClick(const MouseEventWithHitTestResults& event)
index 2b8abb2..4d366e9 100644 (file)
@@ -1,3 +1,13 @@
+2020-01-14  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Enable the offset assertion in HTMLTextFormControlElement::indexForPosition
+        https://bugs.webkit.org/show_bug.cgi?id=205706
+
+        Reviewed by Darin Adler.
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::rangeNearPositionMatchesText):
+
 2020-01-14  Chris Dumez  <cdumez@apple.com>
 
         document.cookie should not do a sync IPC to the network process for iframes that do not have storage access
index 1f81aea..ebe57c6 100644 (file)
@@ -1957,7 +1957,7 @@ void WebPage::storeSelectionForAccessibility(bool shouldStore)
 static RefPtr<Range> rangeNearPositionMatchesText(const VisiblePosition& position, RefPtr<Range> originalRange, const String& matchText, RefPtr<Range> selectionRange)
 {
     auto range = Range::create(selectionRange->ownerDocument(), selectionRange->startPosition(), position.deepEquivalent().parentAnchoredEquivalent());
-    unsigned targetOffset = TextIterator::rangeLength(range.ptr(), true);
+    unsigned targetOffset = TextIterator::rangeLength(range.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     return findClosestPlainText(*selectionRange.get(), matchText, { }, targetOffset);
 }