Bug 188322 – [iOS] Caret disappears after resigning and becoming first responder if active focus state is retained

• Component: HTML Editing
• Status: RESOLVED FIXED

[Wenson Hsieh]

<rdar://problem/42455270>

[Wenson Hsieh]

Began happening after r230745. Causes the caret to disappear in iOS Safari on iPad if the user taps the URL bar, and then the page.

[Wenson Hsieh]

Created attachment 346566 [details]
Patch

[Wenson Hsieh]

Created attachment 346569 [details]
Fix macOS builds

[EWS Watchlist]

Comment on attachment 346569 [details]
Fix macOS builds

Attachment 346569 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8763987

New failing tests:
editing/selection/ios/selection-handles-after-touch-end.html
imported/blink/transitions/unprefixed-transform.html

[EWS Watchlist]

Created attachment 346595 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Wenson Hsieh]

Created attachment 346596 [details]
Tweak Windows TestExpectations

[EWS Watchlist]

Comment on attachment 346596 [details]
Tweak Windows TestExpectations

Attachment 346596 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8764498

New failing tests:
http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-video.html
legacy-animation-engine/imported/blink/transitions/unprefixed-transform.html

[EWS Watchlist]

Created attachment 346597 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Tim Horton]

Comment on attachment 346596 [details]
Tweak Windows TestExpectations

View in context: https://bugs.webkit.org/attachment.cgi?id=346596&action=review

> LayoutTests/ChangeLog:10
> +        Adds a new layout test to verify that (1) selection handles are not shown when selecting a word by long
> +        pressing prior to ending the touch, and (2) selection handles are shown after ending the touch.

V. nice.

[Wenson Hsieh]

Comment on attachment 346596 [details]
Tweak Windows TestExpectations

Thanks for the review!

[WebKit Commit Bot]

Comment on attachment 346596 [details]
Tweak Windows TestExpectations

Clearing flags on attachment: 346596

Committed r234600: <https://trac.webkit.org/changeset/234600>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Wenson Hsieh]

Comment on attachment 346596 [details]
Tweak Windows TestExpectations

View in context: https://bugs.webkit.org/attachment.cgi?id=346596&action=review

> Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm:127
> +    auto expectedCaretRect = CGRectMake(16, 13, 3, 15);

Oops, this test expectation is actually wrong (I missed updating the expected width after adding some logic to convert the caret rect to content view coordinates). I'll put out a fix for this in a bit...

[Wenson Hsieh]

Reopening to attach new patch.

[Wenson Hsieh]

Created attachment 346635 [details]
Adjust the API test

[WebKit Commit Bot]

Comment on attachment 346635 [details]
Adjust the API test

Clearing flags on attachment: 346635

Committed r234614: <https://trac.webkit.org/changeset/234614>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 346635 [details]
Adjust the API test

View in context: https://bugs.webkit.org/attachment.cgi?id=346635&action=review

> Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm:41
> +    return CGRectMake(roundl(rect.origin.x), roundl(rect.origin.y), roundl(rect.size.width), roundl(rect.size.height));

The use of roundl here is peculiar and almost certainly wrong; that function is the version that takes and returns a long double. Why are we converting these doubles to long doubles? I suggest using std::round here, which is overloaded for various types rather than the C round where you have to use the correct name for each type.

Separately, I am really unclear why rounding to integers is the right thing to do. But it’s in a test so I won’t probe too hard on that question.

[Wenson Hsieh]

Comment on attachment 346635 [details]
Adjust the API test

View in context: https://bugs.webkit.org/attachment.cgi?id=346635&action=review

>> Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm:41
>> +    return CGRectMake(roundl(rect.origin.x), roundl(rect.origin.y), roundl(rect.size.width), roundl(rect.size.height));
> 
> The use of roundl here is peculiar and almost certainly wrong; that function is the version that takes and returns a long double. Why are we converting these doubles to long doubles? I suggest using std::round here, which is overloaded for various types rather than the C round where you have to use the correct name for each type.
> 
> Separately, I am really unclear why rounding to integers is the right thing to do. But it’s in a test so I won’t probe too hard on that question.

Got it — I'll change this to std::round.

I'm rounding here because the caret view's frame converted to content view coordinates is very close to (but not exactly) {{ 16, 13 }, { 2, 15 }}.

[Wenson Hsieh]

Reopening to attach new patch.

[Wenson Hsieh]

Created attachment 346682 [details]
Patch

[Sam Weinig]

Comment on attachment 346682 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=346682&action=review

> Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm:36
> +#import <wtf/MathExtras.h>

I think <cmath> would be fine here.

[Wenson Hsieh]

Comment on attachment 346682 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=346682&action=review

>> Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm:36
>> +#import <wtf/MathExtras.h>
> 
> I think <cmath> would be fine here.

👍

[Wenson Hsieh]

https://trac.webkit.org/changeset/234657/webkit(In reply to Sam Weinig from comment #22)
> Comment on attachment 346682 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=346682&action=review
> 
> > Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm:36
> > +#import <wtf/MathExtras.h>
> 
> I think <cmath> would be fine here.

Committed r234657: <https://trac.webkit.org/changeset/234657/webkit>