Return a valid navigation id from the browsingContext.navigate command
Categories
(Remote Protocol :: WebDriver BiDi, task, P2)
Tracking
(firefox117 fixed)
Tracking | Status | |
---|---|---|
firefox117 | --- | fixed |
People
(Reporter: jdescottes, Assigned: jdescottes)
References
(Blocks 1 open bug)
Details
(Whiteboard: [webdriver:m8], [wptsync upstream] [webdriver:relnote])
Attachments
(2 files, 2 obsolete files)
We are landing a basic implementation for browsingContext.navigate
in Bug 1730642, but we don't return a navigation id yet.
This navigation id should be a stable id that can represent the complete navigation. Our CDP implementation uses the channelId from network events, but BrowsingContext's CurrentLoadIdentifier might also be used here. CurrentLoadIdentifier is not yet exposed to chrome JS.
Updated•1 year ago
|
Updated•1 year ago
|
Assignee | ||
Comment 1•10 months ago
|
||
Based on https://bugzilla.mozilla.org/show_bug.cgi?id=1730642#c6 and https://bugzilla.mozilla.org/show_bug.cgi?id=1730642#c7 I tried to poke around BrowsingContext's currentLoadIdentifier
but as far as I can tell it is usually set after we get notified about a STATE_START change in the we progress listener, and I think it gets "unset" quite early as well, so it doesn't seem like I could use this as a navigation id.
For webdriver bidi we need to expose a unique navigation id in various events and commands, so that consumers can know which navigation relates to which events/commands. This navigation id is the one you can find in the specs at https://html.spec.whatwg.org/#navigation-id, but I don't really know how to re-create this in Firefox.
This id is supposed to be available very early in the navigation (for BiDi I think the first observable event which should have it is the one triggered from step 12 of https://html.spec.whatwg.org/#beginning-navigation.
Nika, do you have suggestions here? Is this navigation id something we could implement on top of some other id / object that should be available for any navigation (even those which don't involve the network)? Or would this require more changes on the platform side?
Comment 2•10 months ago
|
||
Unfortunately, as far as I know we don't currently have an implementation of the navigation-id
concept in the browser. We do, in roundabout ways, track navigations from source to destination, but IIRC it's not handled with a consistent UUID identifier, but rather with a bunch of independent mechanisms.
Off the top of my head there's:
- The
nsDocShellLoadState::LoadIdentifier()
(the same one which is used forcurrentLoadIdentifier
)- This isn't available in WebProgressListener notifications, and isn't consistently available in content processes.
- As you mentioned, it's currently cleared when redirecting to the final process, rather than when the load is actually finished, as
DocumentLoadListener
currently fully hands-off control over the load to nsDocLoader/necko once it's been targeted to the final process.
- The
nsIIdentChannel::channelId
- This was added for the devtools netmonitor to track a HttpChannel across processes, and is only implemented for
DocumentChannel
andHttpChannel
, so wouldn't work for any non-standard channels. - I don't think this necessarily persists across redirects, but I can't remember for sure.
- Not currently available in parent-process WebProgressListener notifications, but would perhaps be the easiest to add.
- This was added for the devtools netmonitor to track a HttpChannel across processes, and is only implemented for
- The SHIP-only
LoadingSessionHistoryEntry
'sLoadId
- This is only available with SHIP (which should hopefully be the only session history backend soon)
- I believe is often not set until we're in the parent process
- Not available in WebProgressListener notifications, and not clearly directly connected to them (as those notifications are generally triggered through the DocLoader)
My intuition suggests that the channelId
won't work for your purposes, as it's intended for very different things. In many ways the LoadingSessionHistoryEntry
feels like it might be appropriate, as it should often be available on the DocShell during the final stages of the load, but it won't be present for many error cases, or when starting the load, as we often only fill it in in the parent process, and tell content once it's actually going to load something.
I think enhancing LoadIdentifier()
until it's actually usable for this purpose is probably the cleanest pathway forward here, though it might be tricky to get everything to line up properly across processes. Other than that, most code needing to track a specific navigation tends to track it manually, using progress listeners to tell when it has completed.
Assignee | ||
Comment 3•10 months ago
|
||
Thanks a lot for the detailed answer Nika!
channelId
I don't think this necessarily persists across redirects, but I can't remember for sure.
I don't think it is. I had to use the WrappedChannel's id to get a consistent id across redirects.
It sounds tempting to expose it somehow the parent process webprogresslistener, but that probably won't work for navigations which don't hit the network.
Since there's nothing which perfectly matches at the moment, I think we will move forward with some manual tracking based on webprogresslisteners. We can file another bug to improve LoadIdentifier()
, not sure we will have time to work on it.
Comment 4•10 months ago
|
||
(In reply to Julian Descottes [:jdescottes] from comment #3)
Since there's nothing which perfectly matches at the moment, I think we will move forward with some manual tracking based on webprogresslisteners. We can file another bug to improve
LoadIdentifier()
, not sure we will have time to work on it.
Yeah, whatever mechanism ends up being built even if it uses LoadIdentifier()
will probably need to work based on web progress listeners in some way, so that's probably what I'd reach for as well.
Assignee | ||
Updated•9 months ago
|
Assignee | ||
Comment 5•3 months ago
|
||
Updated•3 months ago
|
Comment 6•3 months ago
|
||
Just as an information, this is a feature Puppeteer would need for correctly returning the network response from page.goto()
.
Updated•3 months ago
|
Updated•3 months ago
|
Assignee | ||
Comment 7•3 months ago
|
||
Depends on D180440
Assignee | ||
Comment 8•3 months ago
|
||
Depends on D181127
Updated•3 months ago
|
Comment 9•3 months ago
|
||
Bug 1841010 will cover the retrieval of the navigation id. As such this bug gets simpler and we agreed on to reduce the points appropriately.
Updated•3 months ago
|
Updated•3 months ago
|
Comment 10•3 months ago
|
||
This is still blocked on bug 1841010 as I wrote in comment 9.
Comment 11•3 months ago
|
||
Comment on attachment 9338632 [details]
Bug 1763122 - [remote] Add a NavigationManager to monitor navigations using JSWindowActors
Revision D180440 was moved to bug 1841010. Setting attachment 9338632 [details] to obsolete.
Comment 12•3 months ago
|
||
Comment on attachment 9339356 [details]
Bug 1763122 - [remote] Start monitoring navigations on WebDriver session startup
Revision D181127 was moved to bug 1841010. Setting attachment 9339356 [details] to obsolete.
Assignee | ||
Comment 13•2 months ago
|
||
Depends on D181129
Comment 14•2 months ago
|
||
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0537362c5280 [bidi] Add navigation id to browsingContext.navigate result r=webdriver-reviewers,jgraham https://hg.mozilla.org/integration/autoland/rev/6edc9521a415 [wdspec] Add tests for navigation id in browsingContext navigate tests r=webdriver-reviewers,jgraham
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/41123 for changes under testing/web-platform/tests
Updated•2 months ago
|
Comment 16•2 months ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0537362c5280
https://hg.mozilla.org/mozilla-central/rev/6edc9521a415
Updated•2 months ago
|
Upstream PR merged by moz-wptsync-bot
Assignee | ||
Updated•2 months ago
|
Assignee | ||
Updated•1 month ago
|
Description
•