fix regression in definition of beforeunload prompts caused by #1040 / #1518 #1573
Conversation
jtnord
requested review from
AutomatedTester,
jgraham,
shs96c and
andreastt
as code owners
The wording of the current entry implies to some that promots from beforeunloaded are always dismissed and thus the promot is in effect never shown and the user will have no chance to interact with it. This interpretation would lead it to be impossible to interact with them and test them - which seems to be the purpose of the webdriver spec, so this would imply that that interpretation is incorrect. This clarifies the wording to say that the promot is dismissed on *subsequent* navigations, thus if a prompt was shown and a navigation attempt was performed then it would be dismissed, however if the cause of the alter was a navigation then the promot will be shown so that the user can interact with it. fixes w3c#1294
There was a problem hiding this comment.
I don't think this is a clarification; it's a total change in the model. As written it implies that something like closing a window can fail, but subsequent calls will succeed. That would need much more support in the spec than this patch provides.
If there's a desire to change the way the protocol works with beforeUnload prompts, the first question is what existing implementations do. If current implementations always follow the current spec it seems likely that unilateral change in the spec will break tests. If there are various behaviours there might be more flexibility to change things without breaking users.
(FWIW I would expect this kind of thing to be fixed in the BiDi version of WebDriver, and clients will get to implement whatever behaviour they like).
Chrome - allows the user to handle them
See above - this is not the case.
Potentially the issue at hand is that the link to |
which could as I understand it have been introduced in 321fff4 because prior to that change there was no definition of |
|
incorrect link in the above. It did indeed used to link not to
|
The "navigation" used in versions of the spec from 33c1f38 used to link to "Go". The previous commit attempted to fix this by removing the link and linking to the generic navigation definition however this was not the intent. SO this was originally introduced in w3c#1040 this fixes this and so now correctly shows when prompts from beforeunload should be implicitly dismissed
| are <a>dismissed</a> implicitly upon <a>navigation</a> | ||
| or <a>close window</a>, | ||
| regardless of the defined <a>user prompt handler</a>. | ||
| are <a>dismissed</a> implicitly upon a <a data-lt="Navigate To">navigation</a> |
jtnord
changed the title
|
@jgraham have you had a chance to look and (in)validate my latest analysis here? Given that Mozilla Firefox / geckodriver is now implementing this "as written" and differently to other browsers based seems pretty scary to me, when as written is caused by an innocent accident. |
|
I realise that #1518 changed |
| are <a>dismissed</a> implicitly upon a <a data-lt="Navigate To">navigation</a> | ||
| or a <a>close window</a>, regardless of the defined <a>user prompt handler</a>. |
There was a problem hiding this comment.
| are <a>dismissed</a> implicitly upon a <a data-lt="Navigate To">navigation</a> | |
| or a <a>close window</a>, regardless of the defined <a>user prompt handler</a>. | |
| are <a>dismissed</a> implicitly upon a WebDriver-initiated <a data-lt="Navigate To"> | |
| navigation</a> or <a>close window</a>, regardless of the defined <a>user prompt | |
| handler</a>. |
There was a problem hiding this comment.
@jgraham I am not sure that this makes clearer, rather the reverse.
Navigate To is a webdriver spec - (it is a POST /session/{session id}/url) so should not be "WebDriver Initiated" - but always initiated by some other code. If some other functionality was linked to "Navigate To" in the spec then this would already be handled by the spec and behaviour - but nothing is linked to this.
The definition of what happens to a prompt created from a
beforeunloadevent unintentionally changed from being implicitly dismissed from explicit navigation to implicit navigation.the link used to go to "Go" but this was renamed in #1040 leaving a dead link.
This link was "fixed" in #1518 but instead of fixing the link to how it was (by linking to the new
"Navigate To"which was renamed earlier) it linked to the default definition ofnavigatewhich is vastly different and covers all navigation implicit or explicit.fixes #1294
Preview | Diff