[Webkit-unassigned] [Bug 175184] WebDriver: implement unhandled prompt behavior

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 7 10:53:12 PDT 2017


https://bugs.webkit.org/show_bug.cgi?id=175184

Brian Burg <bburg at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #317435|review?                     |review+
              Flags|                            |

--- Comment #3 from Brian Burg <bburg at apple.com> ---
Comment on attachment 317435
  --> https://bugs.webkit.org/attachment.cgi?id=317435
Patch

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

r=me

> Source/WebDriver/ChangeLog:16
> +        as part fo the result error message.

Nit: of

> Source/WebDriver/ChangeLog:19
> +        (WebDriver::Session::handleUserPrompts): Check if therre's an active JavaScript dialog and deal with it depeding

Nit: there's

> Source/WebDriver/Session.cpp:167
> +        bool shouldFailOnDismiss = false;

I think it would be cleaner here to use a std::optional<UnhandledPromptBehavior>. The !value() case can go before the switch and you can move the most complicated part of the switch case (the if branch) out of the switch entirely.

> Source/WebDriver/Session.cpp:210
> +            errorResult.setAdditonalErrorData(WTFMove(additonalData));

Nit: setAdditionalErrorData

> Source/WebDriver/Session.cpp:223
> +    handleUserPrompts([this, url, completionHandler = WTFMove(completionHandler)](CommandResult&& result) mutable {

This significantly indents most of the session code. Is there a way we can extract it into a separate method to cut down on async indent noise? (this situation always makes me miss JS promises..)

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170807/41295691/attachment.html>


More information about the webkit-unassigned mailing list