Bug 204257

Summary: Update dismiss-picker-using-keyboard.html test to work on iPad correctly
Product: WebKit Reporter: Megan Gardner <megan_gardner>
Component: New BugsAssignee: Megan Gardner <megan_gardner>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, commit-queue, simon.fraser, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Megan Gardner 2019-11-15 15:34:39 PST
Update dismiss-picker-using-keyboard.html to work on iPad correctly
Comment 1 Megan Gardner 2019-11-15 16:02:35 PST
Created attachment 383659 [details]
Patch
Comment 2 Megan Gardner 2019-11-15 16:37:28 PST
<rdar://problem/57239690>
Comment 3 Wenson Hsieh 2019-11-15 20:53:09 PST
Comment on attachment 383659 [details]
Patch

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

> LayoutTests/resources/ui-helper.js:367
> -                    uiController.didShowKeyboardCallback = function() {
> +                    function clearCallbacksAndScriptComplete() {
> +                        uiController.didShowKeyboardCallback = null;
> +                        uiController.willPresentPopoverCallback = null;
>                          uiController.uiScriptComplete();
> -                    };
> +                    }
> +                }
> +                (function() {
> +                    uiController.didShowKeyboardCallback = clearCallbacksAndScriptComplete;
> +                    uiController.willPresentPopoverCallback = clearCallbacksAndScriptComplete;
>                      uiController.singleTapAtPoint(${x}, ${y}, function() { });
>                  })()`, resolve);

It looks like there's an extra open parenthesis at the beginning of this script. clearCallbacksAndScriptComplete is also scoped to its anonymous function (which is also never invoked), so uiController.didShowKeyboardCallback and uiController.willPresentPopoverCallback won't get set to it. I think you meant to define clearCallbacksAndScriptComplete inside the second anonymous function?

> LayoutTests/resources/ui-helper.js:374
> +        if (!this.isWebKit2() || !this.isIOSFamily())
> +            return this.activateAt(x, y);

I don't think you meant to put this here.

> LayoutTests/resources/ui-helper.js:381
> +                        uiController.waitForKeyboardToHide = null;
> +                        uiController.waitForPopoverToDismiss = null;

Same comments as above.

waitForKeyboardToHide and waitForPopoverToDismiss also don't seem to be attributes on UIScriptController.

> LayoutTests/resources/ui-helper.js:387
> +                    uiController.waitForKeyboardToHide = clearCallbacksAndScriptComplete;
> +                    uiController.waitForPopoverToDismiss = clearCallbacksAndScriptComplete;

Ditto.
Comment 4 Megan Gardner 2019-11-18 13:23:17 PST
Created attachment 383781 [details]
Patch
Comment 5 Wenson Hsieh 2019-11-18 13:31:46 PST
Comment on attachment 383781 [details]
Patch

r=mews
Comment 6 Simon Fraser (smfr) 2019-11-18 13:38:56 PST
Comment on attachment 383781 [details]
Patch

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

> LayoutTests/resources/ui-helper.js:366
> -                    uiController.didShowKeyboardCallback = function() {
> +                    function clearCallbacksAndScriptComplete() {
> +                        uiController.didShowKeyboardCallback = null;
> +                        uiController.willPresentPopoverCallback = null;
>                          uiController.uiScriptComplete();
> -                    };
> +                    }
> +                    uiController.didShowKeyboardCallback = clearCallbacksAndScriptComplete;
> +                    uiController.willPresentPopoverCallback = clearCallbacksAndScriptComplete;
>                      uiController.singleTapAtPoint(${x}, ${y}, function() { });
>                  })()`, resolve);
>          });

This looks the same as waitForInputSessionToDismiss() so can you just call that here?
Comment 7 WebKit Commit Bot 2019-11-18 17:05:22 PST
Comment on attachment 383781 [details]
Patch

Clearing flags on attachment: 383781

Committed r252608: <https://trac.webkit.org/changeset/252608>
Comment 8 WebKit Commit Bot 2019-11-18 17:05:23 PST
All reviewed patches have been landed.  Closing bug.