Bug 196322 - [iPad] Tapping on a popup form control may not show a popover
Summary: [iPad] Tapping on a popup form control may not show a popover
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: WebKit Local Build
Hardware: iPhone / iPad iOS 12
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar, PlatformOnly
Depends on:
Blocks:
 
Reported: 2019-03-27 15:34 PDT by Daniel Bates
Modified: 2019-03-28 09:36 PDT (History)
7 users (show)

See Also:


Attachments
Patch (7.49 KB, patch)
2019-03-27 15:50 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (7.87 KB, patch)
2019-03-27 15:55 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (7.80 KB, patch)
2019-03-27 16:05 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (7.80 KB, patch)
2019-03-27 16:08 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews202 for win-future (12.92 MB, application/zip)
2019-03-27 18:10 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews122 for ios-simulator-wk2 (17.21 MB, application/zip)
2019-03-27 18:24 PDT, Build Bot
no flags Details
To land (6.37 KB, patch)
2019-03-28 09:32 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
To land (6.37 KB, patch)
2019-03-28 09:35 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2019-03-27 15:34:15 PDT
Seen on iOS with ToT.

Steps to reproduce:

Perform on an iPad.

1. Open Settings > General > Keyboards and disable shortcuts.
2. Open Safari and visit <data:text/html,<select><option>First</option><option>Second</option></select>>.
3. Tap the <select> form control.

Then nothing happens. But a popover should open.
Comment 1 Daniel Bates 2019-03-27 15:34:40 PDT
<rdar://problem/49229632>
Comment 2 Daniel Bates 2019-03-27 15:50:44 PDT
Created attachment 366118 [details]
Patch
Comment 3 Daniel Bates 2019-03-27 15:55:38 PDT
Created attachment 366120 [details]
Patch
Comment 4 Daniel Bates 2019-03-27 15:58:48 PDT
Cannot think of a way to write a test for this without going out of my way to programmatically toggle Keyboard Shortcuts :(
Comment 5 Daniel Bates 2019-03-27 16:05:21 PDT
Created attachment 366121 [details]
Patch

Rebased since <https://bugs.webkit.org/show_bug.cgi?id=196272> has not landed yet.
Comment 6 Daniel Bates 2019-03-27 16:05:50 PDT
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.
Comment 7 Daniel Bates 2019-03-27 16:08:11 PDT
Created attachment 366122 [details]
Patch
Comment 8 Build Bot 2019-03-27 18:09:50 PDT Comment hidden (obsolete)
Comment 9 Build Bot 2019-03-27 18:10:02 PDT Comment hidden (obsolete)
Comment 10 Build Bot 2019-03-27 18:24:55 PDT
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
Comment 11 Build Bot 2019-03-27 18:24:57 PDT
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
Comment 12 Wenson Hsieh 2019-03-27 18:55:26 PDT
(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).
Comment 13 Wenson Hsieh 2019-03-27 18:59:24 PDT
(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...
Comment 14 Daniel Bates 2019-03-27 19:24:22 PDT
(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.
Comment 15 Daniel Bates 2019-03-27 19:25:29 PDT
(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.
Comment 16 Wenson Hsieh 2019-03-27 19:28:59 PDT
(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 17 Daniel Bates 2019-03-28 09:26:25 PDT
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.
Comment 18 Daniel Bates 2019-03-28 09:32:09 PDT
Created attachment 366179 [details]
To land
Comment 19 Daniel Bates 2019-03-28 09:35:07 PDT
Created attachment 366180 [details]
To land
Comment 20 Daniel Bates 2019-03-28 09:36:25 PDT
Committed r243606: <https://trac.webkit.org/changeset/243606>