w3c / webdriver-bidi

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

Clarify `script.Target` behavior when both `realm` and `context` are provided #635

Open Lightning00Blade opened 6 months ago

Lightning00Blade commented 6 months ago

The spec defines this behavior here Reading though it we first check for context + ?sandbox then for realm but as a user I may send both context + realm. The can be done as an oversight as in sending script.Source for the target in script.evaluate/script.callFunction. Currently Firefox fails while Chromium works when both are provided. We should align this on the spec level.

lutien commented 6 months ago

For the context, there was already some discussion of this topic in this issue: https://github.com/w3c/webdriver-bidi/issues/274.

whimboo commented 6 months ago

Here a comment from @jgraham that he gave on Matrix:

Spec wise it's pretty obvious that we ought to have required a type field here as we do for other cases where we want to be explicit about which variant of a sum type is being used. I think I didn't realise at the time that CDDL ignores additional fields.

Unfortunately I think it's probably a lot of work to add a type field now; it will at least break many wdspec tests. But maybe it's better to take the hit.

In any case the spec is clear about what the behaviour should be: ignore the realm field if there's a context field.

We are going to fix the behavior in Firefox via https://bugzilla.mozilla.org/show_bug.cgi?id=1873688.

But I'll keep the issue open so we can further discuss if adding a type field would still be possible to get done. IMHO each browser could just add it for now so it matches the Extensible and later when the flag reached each browser release we could make it a requirement in the BiDi spec? Would that work?

whimboo commented 5 months ago

@jgraham do you think that the idea from my last comment to have an optional type field for now and which will be changed to required later could work?