Changeset 266909 in webkit


Ignore:
Timestamp:
Sep 10, 2020 11:10:56 PM (4 years ago)
Author:
Megan Gardner
Message:

Source/WebCore:
Text replacements at the beginning of a second line are replaced too early
https://bugs.webkit.org/show_bug.cgi?id=216327
<rdar://problem/68170353>

Reviewed by Darin Adler.

In the changes in r258871, using SimpleRanges instead of Range causing some side effects
when the replacements at the beginning of lines. The ranges that we are counting are backwards
and the return characters are being counted instead of being ignored. There is almost
certainly a better fix than this, but this patch restores the original logic that
was present when Range was being used, until a better fix can be worked out.

Test: editing/spelling/text-replacement-first-word-second-line.html

  • editing/Editor.cpp:

(WebCore::Editor::markAndReplaceFor):

  • editing/TextCheckingHelper.cpp:

(WebCore::TextCheckingParagraph::automaticReplacementStart const):
(WebCore::TextCheckingParagraph::automaticReplacementLength const):

  • editing/TextCheckingHelper.h:

LayoutTests:
Overlapping text replacements at the beginning of a line are replaced too early
https://bugs.webkit.org/show_bug.cgi?id=216327

Reviewed by Darin Adler.

  • editing/spelling/text-replacement-first-word-second-line-expected.txt: Added.
  • editing/spelling/text-replacement-first-word-second-line.html: Added.
Location:
trunk
Files:
2 added
8 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r266906 r266909  
     12020-09-10  Megan Gardner  <megan_gardner@apple.com>
     2
     3        Overlapping text replacements at the beginning of a line are replaced too early
     4        https://bugs.webkit.org/show_bug.cgi?id=216327
     5
     6        Reviewed by Darin Adler.
     7
     8        * editing/spelling/text-replacement-first-word-second-line-expected.txt: Added.
     9        * editing/spelling/text-replacement-first-word-second-line.html: Added.
     10
    1112020-09-10  Lauro Moura  <lmoura@igalia.com>
    212
  • trunk/LayoutTests/platform/gtk/TestExpectations

    r266906 r266909  
    992992# Requires support for testing text replacement
    993993editing/spelling/text-replacement-after-typing-to-word.html [ Skip ]
     994editing/spelling/text-replacement-first-word-second-line.html [ Skip ]
    994995
    995996# MediaRecorder is not currently implemented
  • trunk/LayoutTests/platform/ios/TestExpectations

    r266743 r266909  
    128128# Requires support for testing text replacement
    129129editing/spelling/text-replacement-after-typing-to-word.html [ Skip ]
     130editing/spelling/text-replacement-first-word-second-line.html [ Skip ]
    130131
    131132# UIKit draws selection on iOS
  • trunk/LayoutTests/platform/wincairo/TestExpectations

    r266365 r266909  
    700700editing/spelling/markers.html [ Skip ]
    701701editing/spelling/text-replacement-after-typing-to-word.html [ Skip ]
     702editing/spelling/text-replacement-first-word-second-line.html [ Skip ]
    702703editing/spelling/toggle-spellchecking.html [ Skip ]
    703704
  • trunk/Source/WebCore/ChangeLog

    r266904 r266909  
     12020-09-10  Megan Gardner  <megan_gardner@apple.com>
     2
     3        Text replacements at the beginning of a second line are replaced too early
     4        https://bugs.webkit.org/show_bug.cgi?id=216327
     5        <rdar://problem/68170353>
     6
     7        Reviewed by Darin Adler.
     8
     9        In the changes in r258871, using SimpleRanges instead of Range causing some side effects
     10        when the replacements at the beginning of lines. The ranges that we are counting are backwards
     11        and the return characters are being counted instead of being ignored. There is almost
     12        certainly a better fix than this, but this patch restores the original logic that
     13        was present when Range was being used, until a better fix can be worked out.
     14
     15        Test: editing/spelling/text-replacement-first-word-second-line.html
     16
     17        * editing/Editor.cpp:
     18        (WebCore::Editor::markAndReplaceFor):
     19        * editing/TextCheckingHelper.cpp:
     20        (WebCore::TextCheckingParagraph::automaticReplacementStart const):
     21        (WebCore::TextCheckingParagraph::automaticReplacementLength const):
     22        * editing/TextCheckingHelper.h:
     23
    1242020-09-10  Myles C. Maxfield  <mmaxfield@apple.com>
    225
  • trunk/Source/WebCore/editing/Editor.cpp

    r266805 r266909  
    28002800        auto resultLength = results[i].range.length;
    28012801        auto resultEndLocation = resultLocation + resultLength;
    2802         auto automaticReplacementEndLocation = paragraph.automaticReplacementStart() + paragraph.automaticReplacementLength() + offsetDueToReplacement;
     2802        auto automaticReplacementStartLocation = paragraph.automaticReplacementStart(true);
     2803        auto automaticReplacementEndLocation = automaticReplacementStartLocation + paragraph.automaticReplacementLength(true) + offsetDueToReplacement;
    28032804        const String& replacement = results[i].replacement;
    28042805        bool resultEndsAtAmbiguousBoundary = useAmbiguousBoundaryOffset && selectionOffset - 1 <= resultEndLocation;
     
    28252826                }
    28262827            }
    2827         } else if (resultEndLocation <= automaticReplacementEndLocation && resultEndLocation >= paragraph.automaticReplacementStart()
     2828        } else if (automaticReplacementStartLocation <= resultEndLocation && resultEndLocation <= automaticReplacementEndLocation
    28282829            && isAutomaticTextReplacementType(resultType)) {
    28292830            // In this case the result range just has to touch the automatic replacement range, so we can handle replacing non-word text such as punctuation.
  • trunk/Source/WebCore/editing/TextCheckingHelper.cpp

    r266295 r266909  
    209209}
    210210
    211 uint64_t TextCheckingParagraph::automaticReplacementStart() const
    212 {
     211uint64_t TextCheckingParagraph::automaticReplacementStart(bool oldBehaviour) const
     212{
     213    if (oldBehaviour && is_gt(documentOrder(paragraphRange().start, m_automaticReplacementRange.start)))
     214        return 0;
     215   
    213216    if (!m_automaticReplacementStart)
    214217        m_automaticReplacementStart = characterCount({ paragraphRange().start, m_automaticReplacementRange.start });
     
    216219}
    217220
    218 uint64_t TextCheckingParagraph::automaticReplacementLength() const
    219 {
     221uint64_t TextCheckingParagraph::automaticReplacementLength(bool oldBehaviour) const
     222{
     223    if (oldBehaviour && is_gt(documentOrder(paragraphRange().start, m_automaticReplacementRange.start)))
     224        return characterCount({ paragraphRange().start, m_automaticReplacementRange.end });
     225   
    220226    if (!m_automaticReplacementLength)
    221227        m_automaticReplacementLength = characterCount(m_automaticReplacementRange);
  • trunk/Source/WebCore/editing/TextCheckingHelper.h

    r265039 r266909  
    5555    StringView checkingSubstring() const { return text().substring(checkingStart(), checkingLength()); }
    5656
    57     uint64_t automaticReplacementStart() const;
    58     uint64_t automaticReplacementLength() const;
     57    uint64_t automaticReplacementStart(bool = false) const;
     58    uint64_t automaticReplacementLength(bool = false) const;
    5959
    6060    bool checkingRangeMatches(CharacterRange range) const { return range.location == checkingStart() && range.length == checkingLength(); }
Note: See TracChangeset for help on using the changeset viewer.