Bug 196322 – [iPad] Tapping on a popup form control may not show a popover

• Status: RESOLVED
• Resolution: FIXED
• Component: UI Events

[Daniel Bates]

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.

[Daniel Bates]

<rdar://problem/49229632>

[Daniel Bates]

Created attachment 366118 [details]
Patch

[Daniel Bates]

Created attachment 366120 [details]
Patch

[Daniel Bates]

Cannot think of a way to write a test for this without going out of my way to programmatically toggle Keyboard Shortcuts :(

[Daniel Bates]

Created attachment 366121 [details]
Patch

Rebased since <https://bugs.webkit.org/show_bug.cgi?id=196272> has not landed yet.

[Daniel Bates]

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.

[Daniel Bates]

Created attachment 366122 [details]
Patch

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[Wenson Hsieh]

(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).

[Wenson Hsieh]

(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...

[Daniel Bates]

(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.

[Daniel Bates]

(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.

[Wenson Hsieh]

(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.

[Daniel Bates]

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.

[Daniel Bates]

Created attachment 366179 [details]
To land

[Daniel Bates]

Created attachment 366180 [details]
To land

[Daniel Bates]

Committed r243606: <https://trac.webkit.org/changeset/243606>