Changeset 256563 in webkit


Ignore:
Timestamp:
Feb 13, 2020 3:21:13 PM (4 years ago)
Author:
commit-queue@webkit.org
Message:

Unreviewed, rolling out r254557.
https://bugs.webkit.org/show_bug.cgi?id=207725

The assert is correct, but unfortunately it will alwasy fail
since there is an existing bug in
HTMLTextFormControlElement::indexForPosition(). See bug
#207724 for more details. (Requested by dydz on #webkit).

Reverted changeset:

"Enable the offset assertion in
HTMLTextFormControlElement::indexForPosition"
https://bugs.webkit.org/show_bug.cgi?id=205706
https://trac.webkit.org/changeset/254557

Location:
trunk/Source
Files:
18 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r256536 r256563  
     12020-02-13  Commit Queue  <commit-queue@webkit.org>
     2
     3        Unreviewed, rolling out r254557.
     4        https://bugs.webkit.org/show_bug.cgi?id=207725
     5
     6        The assert is correct, but unfortunately it will alwasy fail
     7        since there is an existing bug in
     8        HTMLTextFormControlElement::indexForPosition(). See bug
     9        #207724 for more details. (Requested by dydz on #webkit).
     10
     11        Reverted changeset:
     12
     13        "Enable the offset assertion in
     14        HTMLTextFormControlElement::indexForPosition"
     15        https://bugs.webkit.org/show_bug.cgi?id=205706
     16        https://trac.webkit.org/changeset/254557
     17
    1182020-02-13  Eric Carlson  <eric.carlson@apple.com>
    219
  • trunk/Source/WebCore/accessibility/AXObjectCache.cpp

    r256442 r256563  
    19381938   
    19391939    auto range = Range::create(m_document, startPosition, originalRange->startPosition());
    1940     unsigned targetOffset = TextIterator::rangeLength(range.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     1940    unsigned targetOffset = TextIterator::rangeLength(range.ptr(), true);
    19411941    return findClosestPlainText(searchRange.get(), matchText, { }, targetOffset);
    19421942}
  • trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp

    r255167 r256563  
    20332033    // We need to consider replaced elements for GTK, as they will be
    20342034    // presented with the 'object replacement character' (0xFFFC).
    2035     return WebCore::indexForVisiblePosition(*node, position, { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     2035    bool forSelectionPreservation = true;
    20362036#else
    2037     return WebCore::indexForVisiblePosition(*node, position);
     2037    bool forSelectionPreservation = false;
    20382038#endif
     2039
     2040    return WebCore::indexForVisiblePosition(*node, position, forSelectionPreservation);
    20392041}
    20402042
  • trunk/Source/WebCore/accessibility/atk/WebKitAccessibleHyperlink.cpp

    r254557 r256563  
    155155{
    156156    // This is going to be the actual length in most of the cases
    157     int baseLength = TextIterator::rangeLength(range, { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     157    int baseLength = TextIterator::rangeLength(range, true);
    158158
    159159    // Check whether the current hyperlink belongs to a list item.
  • trunk/Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp

    r254557 r256563  
    411411    // Set values for start offsets and calculate initial range length.
    412412    // These values might be adjusted later to cover special cases.
    413     startOffset = webCoreOffsetToAtkOffset(coreObject, TextIterator::rangeLength(rangeInParent.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements }));
     413    startOffset = webCoreOffsetToAtkOffset(coreObject, TextIterator::rangeLength(rangeInParent.ptr(), true));
    414414    auto nodeRange = Range::create(node->document(), nodeRangeStart, nodeRangeEnd);
    415     int rangeLength = TextIterator::rangeLength(nodeRange.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     415    int rangeLength = TextIterator::rangeLength(nodeRange.ptr(), true);
    416416
    417417    // Special cases that are only relevant when working with *_END boundaries.
  • trunk/Source/WebCore/accessibility/atk/WebKitAccessibleUtil.cpp

    r254557 r256563  
    248248    else if (!isStartOfLine(endPosition)) {
    249249        RefPtr<Range> range = makeRange(startPosition, endPosition.previous());
    250         offset = TextIterator::rangeLength(range.get(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements }) + 1;
     250        offset = TextIterator::rangeLength(range.get(), true) + 1;
    251251    } else {
    252252        RefPtr<Range> range = makeRange(startPosition, endPosition);
    253         offset = TextIterator::rangeLength(range.get(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     253        offset = TextIterator::rangeLength(range.get(), true);
    254254    }
    255255
  • trunk/Source/WebCore/editing/ApplyStyleCommand.cpp

    r254557 r256563  
    253253    auto startRange = Range::create(document(), firstPositionInNode(scope), visibleStart.deepEquivalent().parentAnchoredEquivalent());
    254254    auto endRange = Range::create(document(), firstPositionInNode(scope), visibleEnd.deepEquivalent().parentAnchoredEquivalent());
    255     int startIndex = TextIterator::rangeLength(startRange.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
    256     int endIndex = TextIterator::rangeLength(endRange.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     255    int startIndex = TextIterator::rangeLength(startRange.ptr(), true);
     256    int endIndex = TextIterator::rangeLength(endRange.ptr(), true);
    257257
    258258    VisiblePosition paragraphStart(startOfParagraph(visibleStart));
     
    286286   
    287287    {
    288         auto startRange = TextIterator::rangeFromLocationAndLength(scope, startIndex, 0, { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
    289         auto endRange = TextIterator::rangeFromLocationAndLength(scope, endIndex, 0, { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     288        auto startRange = TextIterator::rangeFromLocationAndLength(scope, startIndex, 0, true);
     289        auto endRange = TextIterator::rangeFromLocationAndLength(scope, endIndex, 0, true);
    290290        if (startRange && endRange)
    291291            updateStartEnd(startRange->startPosition(), endRange->startPosition());
  • trunk/Source/WebCore/editing/CompositeEditCommand.cpp

    r254557 r256563  
    14341434            if (startInParagraph) {
    14351435                auto startRange = Range::create(document(), startOfParagraphToMove.deepEquivalent().parentAnchoredEquivalent(), visibleStart.deepEquivalent().parentAnchoredEquivalent());
    1436                 startIndex = TextIterator::rangeLength(startRange.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     1436                startIndex = TextIterator::rangeLength(startRange.ptr(), true);
    14371437            }
    14381438
     
    14401440            if (endInParagraph) {
    14411441                auto endRange = Range::create(document(), startOfParagraphToMove.deepEquivalent().parentAnchoredEquivalent(), visibleEnd.deepEquivalent().parentAnchoredEquivalent());
    1442                 endIndex = TextIterator::rangeLength(endRange.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     1442                endIndex = TextIterator::rangeLength(endRange.ptr(), true);
    14431443            }
    14441444        }
     
    15111511
    15121512    auto startToDestinationRange = Range::create(document(), firstPositionInNode(editableRoot.get()), destination.deepEquivalent().parentAnchoredEquivalent());
    1513     destinationIndex = TextIterator::rangeLength(startToDestinationRange.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     1513    destinationIndex = TextIterator::rangeLength(startToDestinationRange.ptr(), true);
    15141514
    15151515    setEndingSelection(VisibleSelection(destination, originalIsDirectional));
     
    15331533        // in a call to rangeFromLocationAndLength with a location past the end
    15341534        // of the document (which will return null).
    1535         RefPtr<Range> start = TextIterator::rangeFromLocationAndLength(editableRoot.get(), destinationIndex + startIndex, 0,
    1536             { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
    1537         RefPtr<Range> end = TextIterator::rangeFromLocationAndLength(editableRoot.get(), destinationIndex + endIndex, 0,
    1538             { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     1535        RefPtr<Range> start = TextIterator::rangeFromLocationAndLength(editableRoot.get(), destinationIndex + startIndex, 0, true);
     1536        RefPtr<Range> end = TextIterator::rangeFromLocationAndLength(editableRoot.get(), destinationIndex + endIndex, 0, true);
    15391537        if (start && end)
    15401538            setEndingSelection(VisibleSelection(start->startPosition(), end->startPosition(), DOWNSTREAM, originalIsDirectional));
  • trunk/Source/WebCore/editing/Editing.cpp

    r254557 r256563  
    10851085
    10861086    auto range = Range::create(document, firstPositionInNode(scope.get()), position.parentAnchoredEquivalent());
    1087     return TextIterator::rangeLength(range.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     1087    return TextIterator::rangeLength(range.ptr(), true);
    10881088}
    10891089
    10901090// FIXME: Merge this function with the one above.
    1091 int indexForVisiblePosition(Node& node, const VisiblePosition& visiblePosition, OptionSet<TextIteratorLengthOption> options)
     1091int indexForVisiblePosition(Node& node, const VisiblePosition& visiblePosition, bool forSelectionPreservation)
    10921092{
    10931093    auto range = Range::create(node.document(), firstPositionInNode(&node), visiblePosition.deepEquivalent().parentAnchoredEquivalent());
    1094     return TextIterator::rangeLength(range.ptr(), options);
     1094    return TextIterator::rangeLength(range.ptr(), forSelectionPreservation);
    10951095}
    10961096
     
    11071107VisiblePosition visiblePositionForIndex(int index, ContainerNode* scope)
    11081108{
    1109     auto range = TextIterator::rangeFromLocationAndLength(scope, index, 0, { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     1109    auto range = TextIterator::rangeFromLocationAndLength(scope, index, 0, true);
    11101110    // Check for an invalid index. Certain editing operations invalidate indices because
    11111111    // of problems with TextIteratorEmitsCharactersBetweenAllVisiblePositions.
  • trunk/Source/WebCore/editing/Editing.h

    r254557 r256563  
    2727
    2828#include "Position.h"
    29 #include "TextIteratorBehavior.h"
    3029#include <wtf/Forward.h>
    3130#include <wtf/HashSet.h>
    32 #include <wtf/OptionSet.h>
    3331#include <wtf/unicode/CharacterNames.h>
    3432
     
    152150
    153151WEBCORE_EXPORT int indexForVisiblePosition(const VisiblePosition&, RefPtr<ContainerNode>& scope);
    154 int indexForVisiblePosition(Node&, const VisiblePosition&, OptionSet<TextIteratorLengthOption> = { });
     152int indexForVisiblePosition(Node&, const VisiblePosition&, bool forSelectionPreservation);
    155153WEBCORE_EXPORT VisiblePosition visiblePositionForPositionWithOffset(const VisiblePosition&, int offset);
    156154WEBCORE_EXPORT VisiblePosition visiblePositionForIndex(int index, ContainerNode* scope);
  • trunk/Source/WebCore/editing/TextIterator.cpp

    r254557 r256563  
    23972397// --------
    23982398
    2399 static TextIteratorBehavior behaviorFromLegnthOptions(OptionSet<TextIteratorLengthOption> options)
    2400 {
    2401     TextIteratorBehavior behavior = TextIteratorDefaultBehavior;
    2402     if (options.contains(TextIteratorLengthOption::GenerateSpacesForReplacedElements))
    2403         behavior |= TextIteratorEmitsCharactersBetweenAllVisiblePositions;
    2404     if (options.contains(TextIteratorLengthOption::IgnoreVisibility))
    2405         behavior |= TextIteratorIgnoresStyleVisibility;
    2406     return behavior;
    2407 }
    2408 
    2409 int TextIterator::rangeLength(const Range* range, OptionSet<TextIteratorLengthOption> options)
     2399int TextIterator::rangeLength(const Range* range, bool forSelectionPreservation)
    24102400{
    24112401    unsigned length = 0;
    2412     for (TextIterator it(range, behaviorFromLegnthOptions(options)); !it.atEnd(); it.advance())
     2402    for (TextIterator it(range, forSelectionPreservation ? TextIteratorEmitsCharactersBetweenAllVisiblePositions : TextIteratorDefaultBehavior); !it.atEnd(); it.advance())
    24132403        length += it.text().length();
    24142404    return length;
     
    24292419}
    24302420
    2431 RefPtr<Range> TextIterator::rangeFromLocationAndLength(ContainerNode* scope, int rangeLocation, int rangeLength, OptionSet<TextIteratorLengthOption> options)
     2421RefPtr<Range> TextIterator::rangeFromLocationAndLength(ContainerNode* scope, int rangeLocation, int rangeLength, bool forSelectionPreservation)
    24322422{
    24332423    Ref<Range> resultRange = scope->document().createRange();
     
    24392429    Ref<Range> textRunRange = rangeOfContents(*scope);
    24402430
    2441     TextIterator it(textRunRange.ptr(), behaviorFromLegnthOptions(options));
     2431    TextIterator it(textRunRange.ptr(), forSelectionPreservation ? TextIteratorEmitsCharactersBetweenAllVisiblePositions : TextIteratorDefaultBehavior);
    24422432   
    24432433    // FIXME: the atEnd() check shouldn't be necessary, workaround for <http://bugs.webkit.org/show_bug.cgi?id=6289>.
  • trunk/Source/WebCore/editing/TextIterator.h

    r254557 r256563  
    3232#include "Range.h"
    3333#include "TextIteratorBehavior.h"
    34 #include <wtf/OptionSet.h>
    3534#include <wtf/Vector.h>
    3635#include <wtf/text/StringView.h>
     
    123122    void appendTextToStringBuilder(StringBuilder& builder) const { copyableText().appendToStringBuilder(builder); }
    124123
    125     WEBCORE_EXPORT static int rangeLength(const Range*, OptionSet<TextIteratorLengthOption> = { });
    126     WEBCORE_EXPORT static RefPtr<Range> rangeFromLocationAndLength(ContainerNode* scope, int rangeLocation, int rangeLength, OptionSet<TextIteratorLengthOption> = { });
     124    WEBCORE_EXPORT static int rangeLength(const Range*, bool spacesForReplacedElements = false);
     125    WEBCORE_EXPORT static RefPtr<Range> rangeFromLocationAndLength(ContainerNode* scope, int rangeLocation, int rangeLength, bool spacesForReplacedElements = false);
    127126    WEBCORE_EXPORT static bool getLocationAndLengthFromRange(Node* scope, const Range*, size_t& location, size_t& length);
    128127    WEBCORE_EXPORT static Ref<Range> subrange(Range& entireRange, int characterOffset, int characterCount);
  • trunk/Source/WebCore/editing/TextIteratorBehavior.h

    r254557 r256563  
    6464typedef unsigned short TextIteratorBehavior;
    6565
    66 enum class TextIteratorLengthOption : uint8_t {
    67     GenerateSpacesForReplacedElements = 1 << 0,
    68     IgnoreVisibility = 1 << 1,
    69 };
    70 
    7166} // namespace WebCore
  • trunk/Source/WebCore/editing/ios/DictationCommandIOS.cpp

    r254557 r256563  
    6969    // FIXME: Add the result marker using a Position cached before results are inserted, instead of relying on TextIterators.
    7070    auto rangeToEnd = Range::create(document(), createLegacyEditingPosition((Node *)root, 0), afterResults.deepEquivalent());
    71     int endIndex = TextIterator::rangeLength(rangeToEnd.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     71    int endIndex = TextIterator::rangeLength(rangeToEnd.ptr(), true);
    7272    int startIndex = endIndex - resultLength;
    7373
    7474    if (startIndex >= 0) {
    75         RefPtr<Range> resultRange = TextIterator::rangeFromLocationAndLength(document().documentElement(), startIndex, endIndex,
    76             { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     75        RefPtr<Range> resultRange = TextIterator::rangeFromLocationAndLength(document().documentElement(), startIndex, endIndex, true);
    7776        ASSERT(resultRange); // FIXME: What guarantees this?
    7877        document().markers().addDictationResultMarker(*resultRange, m_metadata);
  • trunk/Source/WebCore/html/HTMLTextFormControlElement.cpp

    r256207 r256563  
    656656    unsigned length = innerTextValue().length();
    657657    index = std::min(index, length); // FIXME: We shouldn't have to call innerTextValue() just to ignore the last LF. See finishText.
     658#if 0
     659    // FIXME: This assertion code was never built, has bit rotted, and needs to be fixed before it can be enabled:
     660    // https://bugs.webkit.org/show_bug.cgi?id=205706.
    658661#if ASSERT_ENABLED
    659662    VisiblePosition visiblePosition = passedPosition;
    660     if (visiblePosition.isNotNull()) {
    661         unsigned indexComputedByVisiblePosition = WebCore::indexForVisiblePosition(*innerText, visiblePosition,
    662             { TextIteratorLengthOption::GenerateSpacesForReplacedElements, TextIteratorLengthOption::IgnoreVisibility });
    663         ASSERT(index == indexComputedByVisiblePosition);
    664     }
     663    unsigned indexComputedByVisiblePosition = 0;
     664    if (visiblePosition.isNotNull())
     665        indexComputedByVisiblePosition = WebCore::indexForVisiblePosition(innerText, visiblePosition, false /* forSelectionPreservation */);
     666    ASSERT(index == indexComputedByVisiblePosition);
     667#endif
    665668#endif
    666669    return index;
  • trunk/Source/WebCore/page/EventHandler.cpp

    r255415 r256563  
    657657{
    658658    auto range = Range::create(start.anchorNode()->document(), start, end);
    659     return TextIterator::rangeLength(range.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     659    return TextIterator::rangeLength(range.ptr(), true);
    660660}
    661661
  • trunk/Source/WebKit/ChangeLog

    r256562 r256563  
     12020-02-13  Commit Queue  <commit-queue@webkit.org>
     2
     3        Unreviewed, rolling out r254557.
     4        https://bugs.webkit.org/show_bug.cgi?id=207725
     5
     6        The assert is correct, but unfortunately it will alwasy fail
     7        since there is an existing bug in
     8        HTMLTextFormControlElement::indexForPosition(). See bug
     9        #207724 for more details. (Requested by dydz on #webkit).
     10
     11        Reverted changeset:
     12
     13        "Enable the offset assertion in
     14        HTMLTextFormControlElement::indexForPosition"
     15        https://bugs.webkit.org/show_bug.cgi?id=205706
     16        https://trac.webkit.org/changeset/254557
     17
    1182020-02-13  Alex Christensen  <achristensen@webkit.org>
    219
  • trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

    r256520 r256563  
    19581958{
    19591959    auto range = Range::create(selectionRange->ownerDocument(), selectionRange->startPosition(), position.deepEquivalent().parentAnchoredEquivalent());
    1960     unsigned targetOffset = TextIterator::rangeLength(range.ptr(), { TextIteratorLengthOption::GenerateSpacesForReplacedElements });
     1960    unsigned targetOffset = TextIterator::rangeLength(range.ptr(), true);
    19611961    return findClosestPlainText(*selectionRange.get(), matchText, { }, targetOffset);
    19621962}
Note: See TracChangeset for help on using the changeset viewer.