Summary: | REGRESSION (r259930): Dictation marker at start of text is removed when added trailing whitespace is collapsed | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||||||
Component: | WebCore Misc. | Assignee: | Darin Adler <darin> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cdumez, darin, esprehn+autocc, ews-watchlist, kangil.han, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar, Regression | ||||||||
Version: | WebKit Local Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=212097 | ||||||||||
Bug Depends on: | 210246 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Daniel Bates
2020-05-19 11:25:42 PDT
Steps to reproduce using iOS Simulator build: 1 Add the following test case: [[ #include "config.h" #if PLATFORM(IOS_FAMILY) #import "PlatformUtilities.h" #import "TestInputDelegate.h" #import "TestWKWebView.h" #import "UIKitSPI.h" #import "WKWebViewConfigurationExtras.h" #import <WebKit/WKWebViewPrivate.h> #import <wtf/unicode/CharacterNames.h> namespace TestWebKitAPI { TEST(InsertTextAlternatives, InsertTrailingSpaceWhitespaceRebalance) { auto *configuration = [WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"WebProcessPlugInWithInternals" configureJSCForTesting:YES]; auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 300, 300) configuration:configuration]); auto inputDelegate = adoptNS([[TestInputDelegate alloc] init]); [inputDelegate setFocusStartsInputSessionPolicyHandler:[] (WKWebView *, id <_WKFocusedElementInfo>) { return _WKFocusStartsInputSessionPolicyAllow; }]; [webView _setInputDelegate:inputDelegate.get()]; [webView synchronouslyLoadHTMLString:@"<body contenteditable='true'></body>"]; [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.body.focus()"]; [[webView textInputContentView] insertText:@"hello" alternatives:@[@"yellow"] style:UITextAlternativeStyleNone]; [[webView textInputContentView] insertText:@" "]; [webView waitForNextPresentationUpdate]; EXPECT_TRUE([[webView objectByEvaluatingJavaScript:@"internals.hasDictationAlternativesMarker(0, 5)"] boolValue]); // hello } } // namespace TestWebKitAPI #endif // PLATFORM(IOS_FAMILY) ]] 2. Run it. Then it fails. Just FYI, I plan to add the test case ^^^ in the patch for bug 212097, flipping the EXPECT_TRUE() to FALSE to make the test pass for now. (In reply to Daniel Bates from comment #3) > Just FYI, I plan to add the test case ^^^ in the patch for bug 212097, > flipping the EXPECT_TRUE() to FALSE to make the test pass for now. OK. But I don’t understand why. That will make it harder for me to land this fix. I guess I can just wait for you to land your fix and then rebase afterward. Created attachment 399761 [details]
Patch
I put a fix here. Dan, feel free to grab the bug if you’d like to take it further. (In reply to Darin Adler from comment #4) > (In reply to Daniel Bates from comment #3) > > Just FYI, I plan to add the test case ^^^ in the patch for bug 212097, > > flipping the EXPECT_TRUE() to FALSE to make the test pass for now. > > OK. But I don’t understand why. That will make it harder for me to land this > fix. Yes, but it makes my workflow easier 🙂. More importantly I am saving you from any agony should that test depend on my work. I don't think it does as I'm typing this... (In reply to Darin Adler from comment #7) > I put a fix here. Dan, feel free to grab the bug if you’d like to take it > further. Thank you! Created attachment 399771 [details]
Patch
Alley-oop
Comment on attachment 399771 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399771&action=review Patch looks good. Needs change log for Tools change I did for you. > Source/WebCore/dom/DocumentMarkerController.cpp:598 > + unsigned targetStartOffset = clampTo<unsigned>(static_cast<int>(marker.startOffset()) + delta); OK as-is. No change needed. Optimal solution would use auto because types in template args <-- is that the right word? > Source/WebCore/dom/DocumentMarkerController.cpp:599 > + unsigned targetEndOffset = clampTo<unsigned>(static_cast<int>(marker.endOffset()) + delta); Ditto. Created attachment 399787 [details]
Patch
Comment on attachment 399787 [details]
Patch
I plan to set commit-queue+ once the EWS tests all pass.
Committed r261903: <https://trac.webkit.org/changeset/261903> All reviewed patches have been landed. Closing bug and clearing flags on attachment 399787 [details]. |