Skip to content

Commit

Permalink
REGRESSION (275503@main): Double space in Mail compose on iOS doesn’t…
Browse files Browse the repository at this point in the history 
… insert period

https://bugs.webkit.org/show_bug.cgi?id=272237
rdar://125852636

Reviewed by Abrar Protyasha and Ryosuke Niwa.

With the changes in 275503@main, we now use `WebCore::findPlainText` to discover the nearby text to
replace, in the case where the replacement range isn't just the last word. While `findPlainText`
(and the underlying `SearchBuffer` machinery) is intended to ignore non-breaking spaces for the
purposes of finding matches, this only works when `UCONFIG_NO_COLLATION` is disabled; otherwise,
searching for the target text `" "` against a single non-breaking space `"0xA0"` will fail to find
any matches.

This subsequently causes autocorrection replacement to fail, in the case where UIKit tries to
replace a space after a sentence with a period, when the user inserts two space characters in a row,
since we fail to match the non-breaking space character right after the word.

To fix this, we align the two find behaviors by moving the `UCONFIG_NO_COLLATION`-specific logic to
handle `nbsp`:

```
inline void SearchBuffer::append(UChar c, bool isStart)
{
    …

    m_buffer[m_cursor] = c == noBreakSpace ? ' ' : foldQuoteMark(c);
}
```

...into `foldQuoteMark`, renaming that helper function to `foldQuoteMarkAndReplaceNoBreakSpace`, and
finally deploying `foldQuoteMarkAndReplaceNoBreakSpace` in the `!UCONFIG_NO_COLLATION` codepath as
well.

Test: editing/input/ios/autocorrection-replaces-space-with-period.html

* LayoutTests/editing/input/ios/autocorrection-replaces-space-with-period-expected.txt: Added.
* LayoutTests/editing/input/ios/autocorrection-replaces-space-with-period.html: Added.
* Source/WebCore/editing/TextIterator.cpp:
(WebCore::foldQuoteMarkAndReplaceNoBreakSpace):
(WebCore::SearchBuffer::append):
(WebCore::foldQuoteMark): Deleted.

Canonical link: https://commits.webkit.org/277151@main
  • Loading branch information
@whsieh
whsieh committed Apr 6, 2024
1 parent f5c2130 commit ac57d4f
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 37 deletions.
10 changes: 10 additions & 0 deletions LayoutTests/editing/input/ios/autocorrection-replaces-space-with-period-expected.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
This test verifies that pressing the Space key twice replaces a space character after 'Hi' with a period.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS editor.textContent became 'Hi.'
PASS successfullyParsed is true

TEST COMPLETE
Hi.
43 changes: 43 additions & 0 deletions LayoutTests/editing/input/ios/autocorrection-replaces-space-with-period.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
<html>
<meta name="viewport" content="width=device-width, initial-scale=1">
<head>
<script src="../../../resources/ui-helper.js"></script>
<script src="../../../resources/js-test.js"></script>
<style>
.editor {
border: 1px solid tomato;
width: 300px;
height: 150px;
font-size: 16px;
}
</style>
<script>
jsTestIsAsync = true;

addEventListener("load", async () => {
description("This test verifies that pressing the Space key twice replaces a space character after 'Hi' with a period.");
await UIHelper.setHardwareKeyboardAttached(false);

editor = document.querySelector(".editor");
await UIHelper.activateElementAndWaitForInputSession(editor);

await UIHelper.callFunctionAndWaitForEvent(async () => {
await UIHelper.typeCharacter(" ");
await UIHelper.ensurePresentationUpdate();
}, editor, "keyup");

await UIHelper.applyAutocorrection(".", " ");
await shouldBecomeEqual("editor.textContent", "'Hi.'");

editor.blur();
await UIHelper.waitForKeyboardToHide();

finishJSTest();
});
</script>
</head>
<body>
<div contenteditable="true" class="editor">Hi</div>
</body>
</html>
76 changes: 39 additions & 37 deletions Source/WebCore/editing/TextIterator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1644,43 +1644,45 @@ StringView WordAwareIterator::text() const

// --------

static inline UChar foldQuoteMark(UChar c)
static inline UChar foldQuoteMarkAndReplaceNoBreakSpace(UChar c)
{
switch (c) {
case hebrewPunctuationGershayim:
case leftDoubleQuotationMark:
case leftLowDoubleQuotationMark:
case rightDoubleQuotationMark:
case leftPointingDoubleAngleQuotationMark:
case rightPointingDoubleAngleQuotationMark:
case doubleHighReversed9QuotationMark:
case doubleLowReversed9QuotationMark:
case reversedDoublePrimeQuotationMark:
case doublePrimeQuotationMark:
case lowDoublePrimeQuotationMark:
case fullwidthQuotationMark:
return '"';
case hebrewPunctuationGeresh:
case leftSingleQuotationMark:
case leftLowSingleQuotationMark:
case rightSingleQuotationMark:
case singleLow9QuotationMark:
case singleLeftPointingAngleQuotationMark:
case singleRightPointingAngleQuotationMark:
case leftCornerBracket:
case rightCornerBracket:
case leftWhiteCornerBracket:
case rightWhiteCornerBracket:
case presentationFormForVerticalLeftCornerBracket:
case presentationFormForVerticalRightCornerBracket:
case presentationFormForVerticalLeftWhiteCornerBracket:
case presentationFormForVerticalRightWhiteCornerBracket:
case fullwidthApostrophe:
case halfwidthLeftCornerBracket:
case halfwidthRightCornerBracket:
return '\'';
default:
return c;
case hebrewPunctuationGershayim:
case leftDoubleQuotationMark:
case leftLowDoubleQuotationMark:
case rightDoubleQuotationMark:
case leftPointingDoubleAngleQuotationMark:
case rightPointingDoubleAngleQuotationMark:
case doubleHighReversed9QuotationMark:
case doubleLowReversed9QuotationMark:
case reversedDoublePrimeQuotationMark:
case doublePrimeQuotationMark:
case lowDoublePrimeQuotationMark:
case fullwidthQuotationMark:
return '"';
case hebrewPunctuationGeresh:
case leftSingleQuotationMark:
case leftLowSingleQuotationMark:
case rightSingleQuotationMark:
case singleLow9QuotationMark:
case singleLeftPointingAngleQuotationMark:
case singleRightPointingAngleQuotationMark:
case leftCornerBracket:
case rightCornerBracket:
case leftWhiteCornerBracket:
case rightWhiteCornerBracket:
case presentationFormForVerticalLeftCornerBracket:
case presentationFormForVerticalRightCornerBracket:
case presentationFormForVerticalLeftWhiteCornerBracket:
case presentationFormForVerticalRightWhiteCornerBracket:
case fullwidthApostrophe:
case halfwidthLeftCornerBracket:
case halfwidthRightCornerBracket:
return '\'';
case noBreakSpace:
return ' ';
default:
return c;
}
}

Expand Down Expand Up @@ -2085,7 +2087,7 @@ inline size_t SearchBuffer::append(StringView text)
ASSERT(usableLength);
m_buffer.grow(oldLength + usableLength);
for (unsigned i = 0; i < usableLength; ++i)
m_buffer[oldLength + i] = foldQuoteMark(text[i]);
m_buffer[oldLength + i] = foldQuoteMarkAndReplaceNoBreakSpace(text[i]);
return usableLength;
}

Expand Down Expand Up @@ -2352,7 +2354,7 @@ inline bool SearchBuffer::atBreak() const

inline void SearchBuffer::append(UChar c, bool isStart)
{
m_buffer[m_cursor] = c == noBreakSpace ? ' ' : foldQuoteMark(c);
m_buffer[m_cursor] = foldQuoteMarkAndReplaceNoBreakSpace(c);
m_isCharacterStartBuffer[m_cursor] = isStart;
if (++m_cursor == m_target.length()) {
m_cursor = 0;
Expand Down

0 comments on commit ac57d4f

Please sign in to comment.