Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should a navigation clean-up the browsing context input state map? #1859

Open
whimboo opened this issue Nov 13, 2024 · 5 comments
Open

Should a navigation clean-up the browsing context input state map? #1859

whimboo opened this issue Nov 13, 2024 · 5 comments

Comments

@whimboo
Copy link
Contributor

whimboo commented Nov 13, 2024

The WebDriver specification expresses the following:

A session has an associated browsing context input state map, which is a weak map with top-level browsing contexts as keys, and input state objects as values. This is initially set to an empty map.

Hereby we we bind the map to a given browsing context. Now consider a situation when an action has already been run and the input cancel list has been updated for actions to be done in release actions to reset the states. But before the call to release actions a navigation takes place.

If it's a same-origin navigation the browsing context is kept and as such the reference to the input state map as well, which means that a call to release actions will run the so far collected undo actions.

If it's a cross-origin navigation the browsing context will be replaced and as such the WeakRef is removed which means that there are no undo actions available anymore.

The above results into an unclear situation and behavior for release actions depending on which type of navigation is run. We probably should define a clear behavior here. Maybe having all browsing context references replaced with navigable might already do it?

@OrKoN
Copy link
Contributor

OrKoN commented Nov 13, 2024

Is the browsing context used here refers to the definition before the HTML spec rewrite? if it is before the spec rewrite, I believe the spec is probably meant to refer to the top-level traversable like the WebDriver BiDi spec (which has been rewritten to account for the changes in the HTML spec). So if this is referring (conceptually) to the top-level traversable, I do not think it is replaced on navigations.

@whimboo
Copy link
Contributor Author

whimboo commented Nov 13, 2024

Ah good call in referring to WebDriver BiDi here. There we actually call:

Let input state be get the input state with session and navigable’s top-level traversable.

And in classic we have:

To get the input state given session and browsing context

So it should indeed be the navigable id and remain intact through navigations.

For elements we nowadays have the following in the WebDriver spec:

A WebDriver session has a navigable seen nodes map which is a weak map between a navigable and a set.

So there is clearly a mismatch. I assume we only have to update the paragraph to something like:

A session has an associated browsing context input state map, which is a weak map between a navigable and an object with input state objects as values. This is initially set to an empty map.

@OrKoN
Copy link
Contributor

OrKoN commented Nov 13, 2024

I assume we only have to update the paragraph to something like

we probably want at some point to audit and rewrite all 355 mentions of browsing context in the WebDriver spec as the HTML spec considers it to be an implementation details and other specs should not directly link to the browsing context concept. Related WebDriver BiDi issue w3c/webdriver-bidi#91

@whimboo
Copy link
Contributor Author

whimboo commented Nov 13, 2024

FYI for the replacement of browsing context to navigable we already have #1696.

Lets quickly discuss it in today's meeting and have a final decision for only this case.

@css-meeting-bot
Copy link
Member

The Browser Testing and Tools Working Group just discussed Should a navigation clean-up the browsing context input state map?.

The full IRC log of that discussion <AutomatedTester> Topic: Should a navigation clean-up the browsing context input state map?
<AutomatedTester> github: https://github.com//issues/1859
<AutomatedTester> whimboo: It looks. like there is a broken understanding between browsing context and navigator
<AutomatedTester> ... this is likely an issue from the move between browsing context and navigator. Do we need to fix everything or can we go fix the actions first by themselves
<orkon> q+
<AutomatedTester> ... when speaking to orkon there are hundreds of cases that may need to be updated in the classic spec. do we want to change one or everything?
<AutomatedTester> ack next
<whimboo> s/this is likely an issue from the move between browsing context and navigator/this is likely an issue from the move between browsing context and navigable
<AutomatedTester> orkon: I think that this is the same problem when we were moving to navigable before with the html spec
<jgraham> q+
<AutomatedTester> ... i think it probably makes sense to at least have a note with browsing context meaning navigable and replace all of them to prevent any confusion between commands
<AutomatedTester> ack next
<AutomatedTester> jgraham: I think every use of browsing context means navigable.
<whimboo> q+
<AutomatedTester> ... <describes if it survives traversible>
<AutomatedTester> ack next
<AutomatedTester> whimboo: given that it should survive navigation then we can close this issue so we don't want to fix this as one item
<orkon> q+
<AutomatedTester> jgraham: the way that orkon did this for bidi was a good way to do it for the classic. It will be a rather large PR
<AutomatedTester> ack next
<AutomatedTester> orkon: unrelated, I noticed that bikeshed allows combining files into a spec. I wonder if we can use this to make it simpler to move things across as there are similarities in the spec.
<AutomatedTester> q?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants