Bug 188107 - [iOS] Spelling suggestions cannot be selected in focused form controls when zoomed in
Summary: [iOS] Spelling suggestions cannot be selected in focused form controls when z...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-07-27 12:12 PDT by Wenson Hsieh
Modified: 2018-10-14 14:41 PDT (History)
9 users (show)

See Also:


Attachments
Patch (6.42 KB, patch)
2018-07-27 12:41 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Adjust layout test (3.77 KB, patch)
2018-07-31 20:24 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Adjust layout test (3.85 KB, patch)
2018-07-31 20:35 PDT, Wenson Hsieh
dbates: review+
Details | Formatted Diff | Diff
Patch for landing (4.18 KB, patch)
2018-07-31 21:43 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2018-07-27 12:12:37 PDT
<rdar://problem/42354250>
Comment 1 Wenson Hsieh 2018-07-27 12:41:53 PDT
Created attachment 345938 [details]
Patch
Comment 2 WebKit Commit Bot 2018-07-31 13:19:15 PDT
Comment on attachment 345938 [details]
Patch

Clearing flags on attachment: 345938

Committed r234436: <https://trac.webkit.org/changeset/234436>
Comment 3 WebKit Commit Bot 2018-07-31 13:19:17 PDT
All reviewed patches have been landed.  Closing bug.
Comment 4 Radar WebKit Bug Importer 2018-07-31 13:20:21 PDT
<rdar://problem/42784249>
Comment 5 Daniel Bates 2018-07-31 18:11:01 PDT
Comment on attachment 345938 [details]
Patch

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

> LayoutTests/fast/forms/ios/click-should-not-suppress-misspelling-expected.txt:3
> +TEST COMPLETE

This output does not look correct. We’re missing the description and this line should be after all other lines.
Comment 6 Wenson Hsieh 2018-07-31 18:53:14 PDT
Comment on attachment 345938 [details]
Patch

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

>> LayoutTests/fast/forms/ios/click-should-not-suppress-misspelling-expected.txt:3
>> +TEST COMPLETE
> 
> This output does not look correct. We’re missing the description and this line should be after all other lines.

This was generated by running the layout test. I'll run it again to double check.

Note that the description is only written when !window.testRunner as a courtesy to anyone running the test manually.
Comment 7 Wenson Hsieh 2018-07-31 19:11:47 PDT
Comment on attachment 345938 [details]
Patch

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

>>> LayoutTests/fast/forms/ios/click-should-not-suppress-misspelling-expected.txt:3
>>> +TEST COMPLETE
>> 
>> This output does not look correct. We’re missing the description and this line should be after all other lines.
> 
> This was generated by running the layout test. I'll run it again to double check.
> 
> Note that the description is only written when !window.testRunner as a courtesy to anyone running the test manually.

I just finished a clean build of trunk iOS simulator (using iOS 12), ran this test, and confirmed that it passes.
Comment 8 Daniel Bates 2018-07-31 19:48:27 PDT
(In reply to Wenson Hsieh from comment #7)
> Comment on attachment 345938 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=345938&action=review
> 
> >>> LayoutTests/fast/forms/ios/click-should-not-suppress-misspelling-expected.txt:3
> >>> +TEST COMPLETE
> >> 
> >> This output does not look correct. We’re missing the description and this line should be after all other lines.
> > 
> > This was generated by running the layout test. I'll run it again to double check.
> > 
> > Note that the description is only written when !window.testRunner as a courtesy to anyone running the test manually.
> 
> I just finished a clean build of trunk iOS simulator (using iOS 12), ran
> this test, and confirmed that it passes.

I didn’t mean to imply there is a correctness issue by my remark. I meant to say that the output of the test does not conform to the expected output of a test that uses js-test. In particular, a js-test should emit a description, plus a line that states that there will be a series of PASS messages, PASS/FAIL output, a successfully parsed line and finally a TEST COMPLETE line.
Comment 9 Daniel Bates 2018-07-31 19:52:22 PDT
One example of the expected output ordering can be seen in <https://trac.webkit.org/browser/webkit/trunk/LayoutTests/fast/events/fire-mousedown-while-pressing-mouse-button-expected.txt>
Comment 10 Wenson Hsieh 2018-07-31 20:23:59 PDT
Reopening to attach new patch.
Comment 11 Wenson Hsieh 2018-07-31 20:24:00 PDT
Created attachment 346248 [details]
Adjust layout test
Comment 12 Wenson Hsieh 2018-07-31 20:24:21 PDT
(In reply to Daniel Bates from comment #9)
> One example of the expected output ordering can be seen in
> <https://trac.webkit.org/browser/webkit/trunk/LayoutTests/fast/events/fire-
> mousedown-while-pressing-mouse-button-expected.txt>

This example lacks a "PASS successfullyParsed is true" line. I used a more recent change as reference (r233387).
Comment 13 Wenson Hsieh 2018-07-31 20:35:12 PDT
Created attachment 346250 [details]
Adjust layout test
Comment 14 Daniel Bates 2018-07-31 21:28:58 PDT
Comment on attachment 346250 [details]
Adjust layout test

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

Thank you Wenson for fixing the test.

> LayoutTests/fast/forms/ios/click-should-not-suppress-misspelling.html:25
> +description(`To manually test, tap the input field to bring up the keyboard, and then tap on a part of the word in

Please start this description with the purpose of the test. For example, in <https://trac.webkit.org/browser/webkit/trunk/LayoutTests/fast/events/fire-mousedown-while-pressing-mouse-button-expected.txt> the description starts with "This test verifies that we fire a mousedown event whenever pressing and holding a mouse button...".

Minor: No need to use grave quotes as we do need any string interpolation. Double quotes are sufficient.
Comment 15 Wenson Hsieh 2018-07-31 21:43:18 PDT
Created attachment 346258 [details]
Patch for landing
Comment 16 WebKit Commit Bot 2018-07-31 22:21:33 PDT
Comment on attachment 346258 [details]
Patch for landing

Clearing flags on attachment: 346258

Committed r234449: <https://trac.webkit.org/changeset/234449>