Repository navigation
Don't serialize a default user prompt handler for BiDi-only sessions - #1975
f3tchcodes wants to merge 1 commit into
Conversation
f3tchcodes
changed the title
ec191f0 to
1b59771
Compare
| <li><p>If <var>flags</var> contains "<code>http</code>", return | ||
| "<code>dismiss and notify</code>". | ||
|
|
||
| <li><p>Return an empty JSON <a>Object</a>. |
There was a problem hiding this comment.
Given that the unhandledPromptBehavior capability is optional in session.CapabilityResponse for BiDi we could as well return null here, and then not add it at all. That's probably more the behavior we would expect here. That way we wouldn't have to pass-through the flag at all given that create a session will handle it.
There was a problem hiding this comment.
Sure, I have updated the serializer to return null when there is no user prompt handler and made Create a Session handle the Classic default while omitting the capability for BiDi only sessions.
1b59771 to
d5971f2
Compare
whimboo
changed the title
whimboo
left a comment
whimboo
left a comment
There was a problem hiding this comment.
It looks close. Thank you @f3tchcodes. I've three inline suggestions to make it easier.
We do not have BiDi-only tests yet so we cannot write a proper wdspec test for it.
| be <a>serialize the user prompt handler</a>. | ||
|
|
||
| <li><p>Set a property on <var>capabilities</var> with the name | ||
| <li><p>If <var>flags</var> contains "<code>http</code>", and |
There was a problem hiding this comment.
I just notice that it might actually be better to move this addition into the http only defaults block below. That way we would have all the relevant classic capability defaults at one place.
| <li><p>If <var>serialized user prompt handler</var> is not null, set a | ||
| property on <var>capabilities</var> with the name | ||
| "<code>unhandledPromptBehavior</code>", and the value <var>serialized | ||
| user prompt handler</var>. |
There was a problem hiding this comment.
The current wording is not wrong but it might probably be better to only serialize the user prompt handler when it's really needed. In case of http we don't need that, and for BiDi only when it's not null. So we could remove the serialized user prompt handler definition and replacing this paragraph with:
<li><p>If the <a>user prompt handler</a> is not null, set a property on
<var>capabilities</var> with the name "<code>unhandledPromptBehavior</code>",
and the value <a>serialize the user prompt handler</a>.
| <ol class=algorithm> | ||
| <li><p>If the <a>user prompt handler</a> is null, return | ||
| "<code>dismiss and notify</code>". | ||
| <li><p>If the <a>user prompt handler</a> is null, return null. |
There was a problem hiding this comment.
With the above changes we would only call this algorithm with a non-null value. So we could remove the line or probably better assert that it's valid.
| <li><p>If the <a>user prompt handler</a> is null, return null. | |
| <li><p>Assert: the <a>user prompt handler</a> is not null. |
Fixes #1834.
For BiDi-only sessions, when
unhandledPromptBehaviorisn't provided,serialize the user prompt handlerreturns"dismiss and notify", which is the Classic WebDriver string format.This change passes the session flags to the serializer so HTTP sessions keep the current behavior, while BiDi only sessions return
{}instead.{}is valid for BiDi because all fields inUserPromptHandlerare optional.This change is
Preview | Diff