w3c / webdriver-bidi

Bidirectional WebDriver protocol for browser automation
https://w3c.github.io/webdriver-bidi/
336 stars 35 forks source link

Define algorithms for the HTML spec to call when a navigable is created or destroyed #708

Closed OrKoN closed 3 weeks ago

OrKoN commented 1 month ago

This PR defines hooks for the HTML spec to use instead of patching the HTML spec. The hooks are defined for navigables as this more closely matches how the WebDriver BiDi spec uses the term browsing context. The destroyed hook gets the browsing context explicitly as the navigable might be disassociated from the active browser context at this point.

Issue: https://github.com/whatwg/html/issues/6194 HTML spec: https://github.com/whatwg/html/pull/10329


Preview | Diff

OrKoN commented 1 month ago

@jgraham what do you think about adding navigable lifecycle hooks into the HTML spec and correspondingly to WebDriver BiDi? I want to try to define the original opener ID via the created hook later.

jgraham commented 1 month ago

Well in general I agree we want to avoid monkeypatching other specs, but have them call directly in to webdriver algorithms when we need to emit an event or similar.

sadym-chromium commented 1 month ago

I believe with this change the section 8.1. HTML is not needed anymore.

sadym-chromium commented 1 month ago

I just figured out: this PR changes the browsingContext.contextDestroyed event behavior. It used to be emitted only for the most-top-level browsing context to be destroyed. Step 1:

If this is not a recursive invocation of this algorithm, call any browsing context tree discarded steps defined in other applicable specifications with browsingContext.

With this change, the event will be emitted for all the nested navigables, effectively emitting the "destroyed" event for all the children.

We can either update the HTML spec (as part of your PR) with a flag if the destroy is recursive (I'm not sure if this will actually work out), or we can accept the changed browsingContext.contextDestroyed logic. Currently both Chrome and Firefox implements it in the new logic way.

OrKoN commented 4 weeks ago

@sadym-chromium see the discussion in https://github.com/whatwg/html/pull/10329#discussion_r1625527017

OrKoN commented 4 weeks ago

@sadym-chromium @jgraham could you please review this PR?

sadym-chromium commented 4 weeks ago

@sadym-chromium see the discussion in whatwg/html#10329 (comment)

This assumption is incorrect. The events are not emitted recursively for the nested navigables. This PR is not a breaking change.

~With this change, the event will be emitted for all the nested navigables, effectively emitting the "destroyed" event for all the children.~

sadym-chromium commented 4 weeks ago

@OrKoN for my understanding of this spec version, the browsingContext.contextDestroyed event should not be emitted for the iframes when the parent navigable is navigated: https://github.com/web-platform-tests/wpt/pull/46562/files