Bug 210010 - RequestTextInputContext.Simple iframe sub-tests may sometimes fail
Summary: RequestTextInputContext.Simple iframe sub-tests may sometimes fail
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-04-04 15:50 PDT by Daniel Bates
Modified: 2020-04-05 09:50 PDT (History)
3 users (show)

See Also:


Attachments
Patch (9.35 KB, patch)
2020-04-04 16:04 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (9.66 KB, patch)
2020-04-04 16:08 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (9.66 KB, patch)
2020-04-04 16:20 PDT, Daniel Bates
darin: review+
Details | Formatted Diff | Diff
[Alt] Patch that changes -performAfterLoading to add user script to main frame only (10.86 KB, patch)
2020-04-04 16:32 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
[Alt] Patch that changes -performAfterLoading to add user script to main frame only (10.88 KB, patch)
2020-04-04 16:35 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
To Land (8.96 KB, patch)
2020-04-05 09:49 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 2020-04-04 15:50:57 PDT
The iframe sub-tests in RequestTextInputContext.Simple are non-deterministic. They may fail if the iframe has not finished loading before the web process hit tests to find the editable elements. To make the these tests more robust they need to wait until the iframe has loaded.
Comment 1 Radar WebKit Bug Importer 2020-04-04 15:51:10 PDT
<rdar://problem/61303697>
Comment 2 Daniel Bates 2020-04-04 16:04:20 PDT
Created attachment 395463 [details]
Patch
Comment 3 Daniel Bates 2020-04-04 16:08:59 PDT
Created attachment 395464 [details]
Patch
Comment 4 Darin Adler 2020-04-04 16:16:46 PDT
Comment on attachment 395464 [details]
Patch

Which test needs the inMainFrameOnly:NO behavior?
Comment 5 Daniel Bates 2020-04-04 16:18:25 PDT
(In reply to Darin Adler from comment #4)
> Comment on attachment 395464 [details]
> Patch
> 
> Which test needs the inMainFrameOnly:NO behavior?

All the existing ones that call -performAfterLoading. I want to invert this relationship (see ChangeLog), but I hope you don't mind I defer for now.
Comment 6 Daniel Bates 2020-04-04 16:19:56 PDT
Comment on attachment 395464 [details]
Patch

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

> Tools/TestWebKitAPI/Tests/WebKitCocoa/RequestTextInputContext.mm:154
> +static webViewLoadHTMLStringAndWaitForDOMLoadEvent(TestWKWebView *webView, NSString *htmlString)

missing void
Comment 7 Daniel Bates 2020-04-04 16:20:35 PDT
Created attachment 395465 [details]
Patch
Comment 8 Darin Adler 2020-04-04 16:21:28 PDT
Comment on attachment 395464 [details]
Patch

I predict we will find no callers that want or need inMainFrameOnly:NO.
Comment 9 Darin Adler 2020-04-04 16:22:39 PDT
Comment on attachment 395465 [details]
Patch

I predict we will find no callers that want or need inMainFrameOnly:NO.

I predict that all the existing callers would work the same or better with inMainFrameOnly:YES.
Comment 10 Daniel Bates 2020-04-04 16:23:26 PDT
(In reply to Darin Adler from comment #8)
> Comment on attachment 395464 [details]
> Patch
> 
> I predict we will find no callers that want or need inMainFrameOnly:NO.

If I was betting, I bet you're right. I just didn't want to rock the boat...
Comment 11 Daniel Bates 2020-04-04 16:24:28 PDT
(In reply to Darin Adler from comment #9)
> Comment on attachment 395465 [details]
> Patch
> 
> I predict we will find no callers that want or need inMainFrameOnly:NO.
> 
> I predict that all the existing callers would work the same or better with
> inMainFrameOnly:YES.

Okay, I'll throw a patch up....
Comment 12 Daniel Bates 2020-04-04 16:24:38 PDT
Thanks for the review!
Comment 13 Daniel Bates 2020-04-04 16:32:40 PDT
Created attachment 395466 [details]
[Alt] Patch that changes -performAfterLoading to add user script to main frame only

Let's see what the bots say...
Comment 14 Daniel Bates 2020-04-04 16:35:15 PDT
Created attachment 395467 [details]
[Alt] Patch that changes -performAfterLoading to add user script to main frame only
Comment 15 Daniel Bates 2020-04-04 16:35:47 PDT
Comment on attachment 395465 [details]
Patch

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

> Tools/TestWebKitAPI/Tests/WebKitCocoa/RequestTextInputContext.mm:170
> +    webViewLoadHTMLStringAndWaitForDOMLoadEvent(webView, applyIframe(@"<input type='text' style='width: 50px; height: 50px;'>"));

Needs a .get() here.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/RequestTextInputContext.mm:175
> +    webViewLoadHTMLStringAndWaitForDOMLoadEvent(webView, applyIframe(@"<textarea style='width: 100px; height: 100px;'></textarea>"));

and here.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/RequestTextInputContext.mm:180
> +    webViewLoadHTMLStringAndWaitForDOMLoadEvent(webView, applyIframe(@"<div contenteditable style='width: 100px; height: 100px;'></div>"));

and here.
Comment 16 Daniel Bates 2020-04-05 09:45:28 PDT
Thanks Simon! @Darin, both bet right, no test failed. Landing the alternative...
Comment 17 Daniel Bates 2020-04-05 09:49:38 PDT
Created attachment 395513 [details]
To Land
Comment 18 Daniel Bates 2020-04-05 09:50:07 PDT
Committed r259551: <https://trac.webkit.org/changeset/259551>