Raise "no such window" error in "dispatching actions" when browsing context (navigable) does no longer exist

[whimboo]

Basically any single action could cause the current (top-level) browsing context to get closed eg. clicking or pressing a key for an element that calls window.close(). The following action in the sequence needs to fail with a no such window error.

I'm going to add tests for classic and BiDi over on https://bugzilla.mozilla.org/show_bug.cgi?id=1932916.

---

Preview | Diff

[OrKoN]

Is there a reason for doing it in the tick action loop and not in https://pr-preview.s3.amazonaws.com/whimboo/webdriver/pull/1861.html#dfn-dispatch-actions ?

[whimboo]

Is there a reason for doing it in the tick action loop and not in https://pr-preview.s3.amazonaws.com/whimboo/webdriver/pull/1861.html#dfn-dispatch-actions ?

That is run only once at the beginning but doesn't cover a discarded navigable due to a single action like key press or mouse click as described in my initial comment.

[OrKoN]

@whimboo right, but in our implementation, for example, dispatch does not wait for the effects to propagate and all actions are dispatched at once so this change is practically no-op (same as checking the navigable existence before all actions).

[OrKoN]

Perhaps, checking it in https://pr-preview.s3.amazonaws.com/whimboo/webdriver/pull/1861.html#dfn-dispatch-actions-inner before dispatch tick actions is called would be better.

[whimboo]

@whimboo right, but in our implementation, for example, dispatch does not wait for the effects to propagate and all actions are dispatched at once so this change is practically no-op (same as checking the navigable existence before all actions).

I think that we should take a view from the users perspective and not how individual browsers have it currently implemented or not.

What should be an appropriate response for such a situation? Should actions only be performed up to the action that caused the navigable to close, and all the rest is ignored? I assume that this is what you currently have. Safari's implementation currently fails as well with no such window error at the moment. So it's Chrome behaving differently.

Perhaps, checking it in https://pr-preview.s3.amazonaws.com/whimboo/webdriver/pull/1861.html#dfn-dispatch-actions-inner before dispatch tick actions is called would be better.

I understand the point regarding a "pointerMove" action with a duration not equal to 0 triggering the window to close, for example, on the first tick. If the platform handles this directly, we indeed have no way to intercept it. Perhaps that’s the approach we should take.

[OrKoN]

I think we want to align here we should specify that after dispatching each of the tick actions implementations should wait until the results of the action have propagated (similarly how it is done for different ticks I believe). The current specification text only says that actions from one tick have to be dispatched at once.

[OrKoN]

Should actions only be performed up to the action that caused the navigable to close, and all the rest is ignored?

IIUC the current spec says that actions (in a single tick) have to be dispatched at once, that means if an action closes the navigable, the rest of the actions is either ignored or applies to the remaining content (I think the browsing context is not necessarily a top-level one in this spec).

For example, in this test page:

<script>
  var iframe = document.createElement('iframe');
  var sameOriginURL = 'iframe-navigations.html';
  window.onmessage = () => {
    iframe.remove();
  }
  window.addEventListener('mousedown', e => console.log('mousedown'), true);
  window.addEventListener('mouseup', e => console.log('mouseup'), true);
</script>
<header>
  <button onclick="iframe.src = sameOriginURL; document.body.append(iframe)">
    Append iframe with a same-origin URL
  </button>
  <button onmousedown="parent.postMessage('test');">
    Post message
  </button>
</header>

when testing in the browser manually, the mouseup event seems to be still dispatched even if the iframe is gone. So I think the user action here succeeds but the equivalent WebDriver gesture would be failing (which might be fine).

[jgraham]

I don't quite understand your proposal @OrKoN.

We can't wait for actions in each tick; the entire model is based around wanting gestures like pinch to work; in that case the two pointerMove actions running concurrently is the entire point.

I think the way the spec is currently written it's clear that once the target browsing context is gone, the action should fail.

Assuming we adopt the proposal to route all actions via the top level browsing context, that continues to be true for the top-level browsing context. I agree it isn't necessarily true for iframes in that model; we'd only need to fail if we were trying to resolve coordinates relative to a now-missing iframe (or an element in that iframe).

However I think this change is ~correct per the current spec and we need to do more work in #1847 to make things work correctly with the dispatch-via-top-level proposal. In particular I think we need to be explicit about the expected event flow; the change there is making the spec inconsistent about where events are actually dispatched. I also think we should consider adding the ability to explicitly specify the context used for each action in that model; if the action chain can interact with multiple documents across multiple iframes, we shouldn't just have a single global context as the target.

[OrKoN]

@jgraham I believe the current version is fine but the previous one was confusing precisely due to the We can't wait for actions in each tick as there would be no way for implementations to reliably detect if the navigable got destroyed as the result of dispatching tick actions.

[whimboo]

Thank you for the review! I'm going to merge this PR now. I don’t think we need feedback from Safari (CC @gsnedders) since it already exhibits the expected behavior introduced by this PR.