- Revision
- 285359
- Author
- wenson_hs...@apple.com
- Date
- 2021-11-05 17:51:16 -0700 (Fri, 05 Nov 2021)
Log Message
[iOS] Mail compose becomes unresponsive after pasting in text and attempting to type
https://bugs.webkit.org/show_bug.cgi?id=232764
rdar://84669661
Reviewed by Geoff Garen.
Source/WebCore:
After some recent changes in UIKit, keyboard code now calls into `-requestDocumentContext:completionHandler:`
after inserting text with the software keyboard, and specifically requests sentence-granularity text context
near the selection. This triggered an existing bug in WebKit, where we hang underneath
`WebPage::requestDocumentEditingContext` when trying to move backwards by sentence granularity, when computing
`contextBeforeStart`; this hang occurs because it's possible for the logic in `nextSentenceBoundaryInDirection`
(inside WebCore) to return a visible position that is in the opposite direction, relative to the given starting
position and direction. In turn, this means we end up revisiting visible positions while moving to the next
sentence boundary.
To prevent this, we make two minor adjustments in editing code. First, in `nextSentenceBoundaryInDirection()`,
we use `result` instead of the given `vp` when trying to find the next sentence boundary, in the case where the
given visible position is not already within a sentence. This was presumably the original intent of this code,
which is to "iterate to the start of the next sentence (when moving downstream) or the end of the previous
sentence (when moving upstream)". However, since `vp` is currently used here instead, we end up moving in the
wrong direction and return the start of the current sentence when moving downstream, or the end of the current
sentence when moving upstream.
Fixing this issue is actually already sufficient in order to address the hangs encountered in the radar;
however, the test case still hits subsequent debug assertions in `moveByGranularityRespectingWordBoundary`, due
to `atBoundaryOfGranularity` never returning `true`, even for visible positions that are at the start or end of
sentences. In lieu of debug assertions, this causes `-requestDocumentContext` to effectively ignore the given
granularity and traverse too far in either direction, due to `atBoundaryOfGranularity` always being `false` (we
avoid infinitely looping and bail because after fixing the first bug above, `nextPosition` eventually becomes
null).
To address this assertion, we make a second tweak to `atBoundaryOfGranularity` that allows us to return `true`
if the given position is already at a sentence boundary (instead of marching to the previous or next sentence
and then moving in the opposite direction). The latter was added in r253561 to fix some corner cases where
`startOfSentence()` or `endOfSentence()` would end up moving us past the next (or previous) sentence, but this
seems to break some existing scenarios where the given position is already at the start or end of the sentence.
Test: DocumentEditingContext.RequestSentencesAfterTextInsertion
* editing/VisibleUnits.cpp:
(WebCore::atBoundaryOfGranularity):
(WebCore::nextSentenceBoundaryInDirection):
See above for more details.
Tools:
Add an API test that exercises the hang (and also verifies that we don't hit any debug assertions in the
process).
* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKitCocoa/DocumentEditingContext.mm:
(TEST):
* TestWebKitAPI/Tests/WebKitCocoa/InsertTextAlternatives.mm:
(TestWebKitAPI::TEST):
* TestWebKitAPI/Tests/WebKitCocoa/simple-editor.html: Added.
* TestWebKitAPI/ios/UIKitSPI.h:
Drive-by fixes: remove some old staging declarations for UIKit SPI that has long been in the internal iOS 15
SDK. Remove the staging categories and definitions, and move them to the non-internal SDK section instead.
Modified Paths
Added Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (285358 => 285359)
--- trunk/Source/WebCore/ChangeLog 2021-11-05 23:55:38 UTC (rev 285358)
+++ trunk/Source/WebCore/ChangeLog 2021-11-06 00:51:16 UTC (rev 285359)
@@ -1,3 +1,50 @@
+2021-11-05 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [iOS] Mail compose becomes unresponsive after pasting in text and attempting to type
+ https://bugs.webkit.org/show_bug.cgi?id=232764
+ rdar://84669661
+
+ Reviewed by Geoff Garen.
+
+ After some recent changes in UIKit, keyboard code now calls into `-requestDocumentContext:completionHandler:`
+ after inserting text with the software keyboard, and specifically requests sentence-granularity text context
+ near the selection. This triggered an existing bug in WebKit, where we hang underneath
+ `WebPage::requestDocumentEditingContext` when trying to move backwards by sentence granularity, when computing
+ `contextBeforeStart`; this hang occurs because it's possible for the logic in `nextSentenceBoundaryInDirection`
+ (inside WebCore) to return a visible position that is in the opposite direction, relative to the given starting
+ position and direction. In turn, this means we end up revisiting visible positions while moving to the next
+ sentence boundary.
+
+ To prevent this, we make two minor adjustments in editing code. First, in `nextSentenceBoundaryInDirection()`,
+ we use `result` instead of the given `vp` when trying to find the next sentence boundary, in the case where the
+ given visible position is not already within a sentence. This was presumably the original intent of this code,
+ which is to "iterate to the start of the next sentence (when moving downstream) or the end of the previous
+ sentence (when moving upstream)". However, since `vp` is currently used here instead, we end up moving in the
+ wrong direction and return the start of the current sentence when moving downstream, or the end of the current
+ sentence when moving upstream.
+
+ Fixing this issue is actually already sufficient in order to address the hangs encountered in the radar;
+ however, the test case still hits subsequent debug assertions in `moveByGranularityRespectingWordBoundary`, due
+ to `atBoundaryOfGranularity` never returning `true`, even for visible positions that are at the start or end of
+ sentences. In lieu of debug assertions, this causes `-requestDocumentContext` to effectively ignore the given
+ granularity and traverse too far in either direction, due to `atBoundaryOfGranularity` always being `false` (we
+ avoid infinitely looping and bail because after fixing the first bug above, `nextPosition` eventually becomes
+ null).
+
+ To address this assertion, we make a second tweak to `atBoundaryOfGranularity` that allows us to return `true`
+ if the given position is already at a sentence boundary (instead of marching to the previous or next sentence
+ and then moving in the opposite direction). The latter was added in r253561 to fix some corner cases where
+ `startOfSentence()` or `endOfSentence()` would end up moving us past the next (or previous) sentence, but this
+ seems to break some existing scenarios where the given position is already at the start or end of the sentence.
+
+ Test: DocumentEditingContext.RequestSentencesAfterTextInsertion
+
+ * editing/VisibleUnits.cpp:
+ (WebCore::atBoundaryOfGranularity):
+ (WebCore::nextSentenceBoundaryInDirection):
+
+ See above for more details.
+
2021-11-05 Patrick Griffis <pgrif...@igalia.com>
[GLIB] Be more careful about calling LowPowerModeNotifier's callback
Modified: trunk/Source/WebCore/editing/VisibleUnits.cpp (285358 => 285359)
--- trunk/Source/WebCore/editing/VisibleUnits.cpp 2021-11-05 23:55:38 UTC (rev 285358)
+++ trunk/Source/WebCore/editing/VisibleUnits.cpp 2021-11-06 00:51:16 UTC (rev 285359)
@@ -1504,9 +1504,17 @@
boundary = useDownstream ? endOfWord(vp, LeftWordIfOnBoundary) : startOfWord(vp, RightWordIfOnBoundary);
break;
- case TextGranularity::SentenceGranularity:
- boundary = useDownstream ? endOfSentence(previousSentencePosition(vp)) : startOfSentence(nextSentencePosition(vp));
+ case TextGranularity::SentenceGranularity: {
+ auto boundaryInDirection = useDownstream ? endOfSentence : startOfSentence;
+ if (vp == boundaryInDirection(vp)) {
+ boundary = vp;
+ break;
+ }
+
+ auto position = useDownstream ? previousSentencePosition(vp) : nextSentencePosition(vp);
+ boundary = boundaryInDirection(position);
break;
+ }
case TextGranularity::LineGranularity:
boundary = vp;
@@ -1683,7 +1691,7 @@
if (result.isNull() || result == vp)
return VisiblePosition();
- result = useDownstream ? startOfSentence(vp) : endOfSentence(vp);
+ result = useDownstream ? startOfSentence(result) : endOfSentence(result);
}
if (result == vp)
Modified: trunk/Tools/ChangeLog (285358 => 285359)
--- trunk/Tools/ChangeLog 2021-11-05 23:55:38 UTC (rev 285358)
+++ trunk/Tools/ChangeLog 2021-11-06 00:51:16 UTC (rev 285359)
@@ -1,3 +1,25 @@
+2021-11-05 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [iOS] Mail compose becomes unresponsive after pasting in text and attempting to type
+ https://bugs.webkit.org/show_bug.cgi?id=232764
+ rdar://84669661
+
+ Reviewed by Geoff Garen.
+
+ Add an API test that exercises the hang (and also verifies that we don't hit any debug assertions in the
+ process).
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/WebKitCocoa/DocumentEditingContext.mm:
+ (TEST):
+ * TestWebKitAPI/Tests/WebKitCocoa/InsertTextAlternatives.mm:
+ (TestWebKitAPI::TEST):
+ * TestWebKitAPI/Tests/WebKitCocoa/simple-editor.html: Added.
+ * TestWebKitAPI/ios/UIKitSPI.h:
+
+ Drive-by fixes: remove some old staging declarations for UIKit SPI that has long been in the internal iOS 15
+ SDK. Remove the staging categories and definitions, and move them to the non-internal SDK section instead.
+
2021-11-05 Ryan Haddad <ryanhad...@apple.com>
[steps.py] Update macOS versions in build_to_name_mapping
Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (285358 => 285359)
--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2021-11-05 23:55:38 UTC (rev 285358)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2021-11-06 00:51:16 UTC (rev 285359)
@@ -1283,6 +1283,7 @@
F4CD74C920FDB49600DE3794 /* TestURLSchemeHandler.mm in Sources */ = {isa = PBXBuildFile; fileRef = F4CD74C820FDB49600DE3794 /* TestURLSchemeHandler.mm */; };
F4CF32802366552200D3AD07 /* EnterKeyHintTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = F4CF327F2366552200D3AD07 /* EnterKeyHintTests.mm */; };
F4CFCDDA249FC9E400527482 /* SpaceOnly.otf in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4CFCDD8249FC9D900527482 /* SpaceOnly.otf */; };
+ F4D060082734A1AB008FA67A /* simple-editor.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4D060072734A08C008FA67A /* simple-editor.html */; };
F4D2986E20FEE7370092D636 /* RunScriptAfterDocumentLoad.mm in Sources */ = {isa = PBXBuildFile; fileRef = F4D2986D20FEE7370092D636 /* RunScriptAfterDocumentLoad.mm */; };
F4D4F3B61E4E2BCB00BB2767 /* DragAndDropSimulatorIOS.mm in Sources */ = {isa = PBXBuildFile; fileRef = F4D4F3B41E4E2BCB00BB2767 /* DragAndDropSimulatorIOS.mm */; };
F4D4F3B91E4E36E400BB2767 /* DragAndDropTestsIOS.mm in Sources */ = {isa = PBXBuildFile; fileRef = F4D4F3B71E4E36E400BB2767 /* DragAndDropTestsIOS.mm */; };
@@ -1722,6 +1723,7 @@
F4FC077720F013B600CA043C /* significant-text-milestone.html in Copy Resources */,
C9B4AD2A1ECA6EBE00F5FEA0 /* silence-long.m4a in Copy Resources */,
1ADBEFE3130C6AA100D61D19 /* simple-accelerated-compositing.html in Copy Resources */,
+ F4D060082734A1AB008FA67A /* simple-editor.html in Copy Resources */,
C0ADBE9612FCA79B00D2C129 /* simple-form.html in Copy Resources */,
33DC8912141955FE00747EF7 /* simple-iframe.html in Copy Resources */,
F4EC8094260D30540010311D /* simple-image-overlay.html in Copy Resources */,
@@ -3180,6 +3182,7 @@
F4CF327F2366552200D3AD07 /* EnterKeyHintTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = EnterKeyHintTests.mm; sourceTree = "<group>"; };
F4CFCDD8249FC9D900527482 /* SpaceOnly.otf */ = {isa = PBXFileReference; lastKnownFileType = file; path = SpaceOnly.otf; sourceTree = "<group>"; };
F4CFCDD9249FC9D900527482 /* Ahem.ttf */ = {isa = PBXFileReference; lastKnownFileType = file; path = Ahem.ttf; sourceTree = "<group>"; };
+ F4D060072734A08C008FA67A /* simple-editor.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "simple-editor.html"; sourceTree = "<group>"; };
F4D2986D20FEE7370092D636 /* RunScriptAfterDocumentLoad.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = RunScriptAfterDocumentLoad.mm; sourceTree = "<group>"; };
F4D4F3B41E4E2BCB00BB2767 /* DragAndDropSimulatorIOS.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = DragAndDropSimulatorIOS.mm; sourceTree = "<group>"; };
F4D4F3B71E4E36E400BB2767 /* DragAndDropTestsIOS.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = DragAndDropTestsIOS.mm; sourceTree = "<group>"; };
@@ -4235,6 +4238,7 @@
F4E3D80720F708E4007B58C5 /* significant-text-milestone-article.html */,
F4FC077620F0108100CA043C /* significant-text-milestone.html */,
C9B4AD291ECA6EA500F5FEA0 /* silence-long.m4a */,
+ F4D060072734A08C008FA67A /* simple-editor.html */,
F4F405BB1D4C0CF8007A9707 /* skinny-autoplaying-video-with-audio.html */,
F4CFCDD8249FC9D900527482 /* SpaceOnly.otf */,
9360270525A3B28E00367670 /* speechrecognition-basic.html */,
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/DocumentEditingContext.mm (285358 => 285359)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/DocumentEditingContext.mm 2021-11-05 23:55:38 UTC (rev 285358)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/DocumentEditingContext.mm 2021-11-06 00:51:16 UTC (rev 285359)
@@ -1326,4 +1326,15 @@
EXPECT_NULL(context.contextAfter);
}
+TEST(DocumentEditingContext, RequestSentencesAfterTextInsertion)
+{
+ auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+ [webView synchronouslyLoadTestPageNamed:@"simple-editor"];
+
+ auto *context = [webView synchronouslyRequestDocumentContext:makeRequest(UIWKDocumentRequestText, UITextGranularitySentence, 1)];
+ EXPECT_NSSTRING_EQ("F", context.contextBefore);
+ EXPECT_NULL(context.selectedText);
+ EXPECT_NSSTRING_EQ("\nThis is a test.", context.contextAfter);
+}
+
#endif // PLATFORM(IOS_FAMILY) && HAVE(UI_WK_DOCUMENT_CONTEXT)
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/InsertTextAlternatives.mm (285358 => 285359)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/InsertTextAlternatives.mm 2021-11-05 23:55:38 UTC (rev 285358)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/InsertTextAlternatives.mm 2021-11-06 00:51:16 UTC (rev 285359)
@@ -62,7 +62,7 @@
EXPECT_WK_STREQ("hello", [[webView textInputContentView] selectedText]);
- auto *alternatives = [(id<UIWKInteractionViewProtocol_Staging66646042>)[webView textInputContentView] alternativesForSelectedText];
+ auto *alternatives = [webView textInputContentView].alternativesForSelectedText;
EXPECT_EQ(1ul, alternatives.count);
EXPECT_WK_STREQ("hello", alternatives[0].primaryString);
EXPECT_EQ(1ul, alternatives[0].alternativeStrings.count);
Added: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/simple-editor.html (0 => 285359)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/simple-editor.html (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/simple-editor.html 2021-11-06 00:51:16 UTC (rev 285359)
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <meta name="viewport" content="width=device-width, initial-scale=1">
+ <script>
+ addEventListener("DOMContentLoaded", () => {
+ let target = document.getElementById("target");
+ getSelection().setPosition(target, 1);
+ });
+ </script>
+</head>
+<body contenteditable>
+Hello world.
+<br id="br1">
+<div style="background: tomato;" id='target'>F</div>
+<br id="br2">
+This is a test.
+</body>
+</html>
Modified: trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h (285358 => 285359)
--- trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h 2021-11-05 23:55:38 UTC (rev 285358)
+++ trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h 2021-11-06 00:51:16 UTC (rev 285359)
@@ -167,6 +167,8 @@
UIWKDocumentRequestRects = 1 << 2,
UIWKDocumentRequestSpatial = 1 << 3,
UIWKDocumentRequestAnnotation = 1 << 4,
+ UIWKDocumentRequestMarkedTextRects = 1 << 5,
+ UIWKDocumentRequestSpatialAndCurrentSelection = 1 << 6,
};
@interface UIWKDocumentRequest : NSObject
@@ -207,6 +209,7 @@
- (void)updateSelectionWithExtentPoint:(CGPoint)point withBoundary:(UITextGranularity)granularity completionHandler:(void (^)(BOOL selectionEndIsMoving))completionHandler;
- (void)selectWordForReplacement;
- (BOOL)textInteractionGesture:(UIWKGestureType)gesture shouldBeginAtPoint:(CGPoint)point;
+- (NSArray<NSTextAlternatives *> *)alternativesForSelectedText;
@property (nonatomic, readonly) NSString *selectedText;
@optional
@@ -267,14 +270,6 @@
#endif // USE(APPLE_INTERNAL_SDK)
-#define UIWKDocumentRequestMarkedTextRects (1 << 5)
-#define UIWKDocumentRequestSpatialAndCurrentSelection (1 << 6)
-
-@protocol UIWKInteractionViewProtocol_Staging66646042 <UIWKInteractionViewProtocol>
-@optional
-- (NSArray<NSTextAlternatives *> *)alternativesForSelectedText;
-@end
-
@interface UITextAutofillSuggestion ()
+ (instancetype)autofillSuggestionWithUsername:(NSString *)username password:(NSString *)password;
@end
