| Summary: | [iPad] Tapping on a popup form control may not show a popover | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||||||||||||||||
| Component: | UI Events | Assignee: | Daniel Bates <dbates> | ||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||
| Severity: | Normal | CC: | bfulgham, ews-watchlist, megan_gardner, rniwa, thorton, webkit-bug-importer, wenson_hsieh | ||||||||||||||||||
| Priority: | P2 | Keywords: | InRadar, PlatformOnly | ||||||||||||||||||
| Version: | WebKit Local Build | ||||||||||||||||||||
| Hardware: | iPhone / iPad | ||||||||||||||||||||
| OS: | iOS 12 | ||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||
|
Description
Daniel Bates
2019-03-27 15:34:15 PDT
Created attachment 366118 [details]
Patch
Created attachment 366120 [details]
Patch
Cannot think of a way to write a test for this without going out of my way to programmatically toggle Keyboard Shortcuts :( Created attachment 366121 [details] Patch Rebased since <https://bugs.webkit.org/show_bug.cgi?id=196272> has not landed yet. Comment on attachment 366121 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366121&action=review > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:4871 > +static RetainPtr<NSObject <WKFormPeripheral>> createInputPeripheralWithView(WebKit::InputType type, WKContentView* view) * is on the wrong side. Created attachment 366122 [details]
Patch
Comment on attachment 366122 [details] Patch Attachment 366122 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/11690050 New failing tests: js/dom/custom-constructors.html Created attachment 366137 [details]
Archive of layout-test-results from ews202 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202 Port: win-future Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Comment on attachment 366122 [details] Patch Attachment 366122 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/11689957 New failing tests: fast/forms/datalist/datalist-show-hide.html Created attachment 366139 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
(In reply to Build Bot from comment #10) > Comment on attachment 366122 [details] > Patch > > Attachment 366122 [details] did not pass ios-sim-ews (ios-simulator-wk2): > Output: https://webkit-queues.webkit.org/results/11689957 > > New failing tests: > fast/forms/datalist/datalist-show-hide.html This failure looks relevant! Let's look into this (it might just be flaky, but I think it directly exercises -inputView — namely, the branch that returns _dataListTextSuggestionsInputView). (In reply to Daniel Bates from comment #4) > Cannot think of a way to write a test for this without going out of my way > to programmatically toggle Keyboard Shortcuts :( I can imagine an API test for this that invokes -[WKContentView inputView] directly... (In reply to Wenson Hsieh from comment #12) > (In reply to Build Bot from comment #10) > > Comment on attachment 366122 [details] > > Patch > > > > Attachment 366122 [details] did not pass ios-sim-ews (ios-simulator-wk2): > > Output: https://webkit-queues.webkit.org/results/11689957 > > > > New failing tests: > > fast/forms/datalist/datalist-show-hide.html > > This failure looks relevant! Let's look into this (it might just be flaky, > but I think it directly exercises -inputView — namely, the branch that > returns _dataListTextSuggestionsInputView). Yep, that was my thought too. (In reply to Wenson Hsieh from comment #13) > (In reply to Daniel Bates from comment #4) > > Cannot think of a way to write a test for this without going out of my way > > to programmatically toggle Keyboard Shortcuts :( > > I can imagine an API test for this that invokes -[WKContentView inputView] > directly... And test what? Nil/non-nil? Maybe looking into the test failure above will give me ideas. (In reply to Daniel Bates from comment #15) > (In reply to Wenson Hsieh from comment #13) > > (In reply to Daniel Bates from comment #4) > > > Cannot think of a way to write a test for this without going out of my way > > > to programmatically toggle Keyboard Shortcuts :( > > > > I can imagine an API test for this that invokes -[WKContentView inputView] > > directly... > > And test what? Nil/non-nil? Maybe looking into the test failure above will > give me ideas. Yeah, that's just about all I think a test like that could check :/ Such an API test would probably not be worth the effort. Comment on attachment 366122 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366122&action=review > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1642 > + if (!hasFocusedElement(_focusedElementInformation) || !_inputPeripheral) Didn't even need to try. Sleep fixes things. This is the line with the bug. We can't early return when we don't have an _inputPeripheral. Why did I make this change? It's because I so want this function to do one thing: only know about input peripheral. The reality is that it can't do one thing we may have a custom view or a data list thingy. Created attachment 366179 [details]
To land
Created attachment 366180 [details]
To land
Committed r243606: <https://trac.webkit.org/changeset/243606> |