1825634 - BiDi load and domContentLoaded events use document baseURI instead of the target URL of the navigation

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, defect, P1)

Description

[Julian Descottes [:jdescottes]]

The BiDi spec defines the url of navigation status at https://w3c.github.io/webdriver-bidi/#module-browsingContext

The URL which is being loaded in the navigation

This navigation status url is used to build the payload of the load and domContentLoaded events, as part of the navigation info.

On the implementation side, we used document.baseURI to retrieve this url: https://searchfox.org/mozilla-central/rev/1157486abdf3245c2d3bf425745314c090745de5/remote/webdriver-bidi/modules/windowglobal/browsingContext.sys.mjs#41

However on pages which define a <base> meta this can significantly differ from the loaded URL.

For instance on https://dashboard.sitespeed.io/d/9NDMzFfMk/page-metrics-desktop?orgId=1&var-base=sitespeed_io&var-path=desktop&var-testname=spa&var-group=dashboard_sitespeed_io&var-page=pageTimingMetricsDefault&var-browser=chrome&var-connectivity=cable&var-function=median&var-resulturl=https:%2F%2Fdata.sitespeed.io%2F&var-screenshottype=jpg , document.baseURI will be https://dashboard.sitespeed.io/.

Comment 1

[Julian Descottes [:jdescottes]]

For reference, switching from document.baseURI to document.URL seems to fix the HAR generation issue spotted over at https://github.com/sitespeedio/browsertime/issues/1925

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: Bug 1825634 - [bidi] Use document URL for load and domContentLoaded events

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1825634 - [wdspec] Add tests for bidi load events with base tags

Depends on D174173

Comment 4

[Julian Descottes [:jdescottes]]

James,

Can you confirm we should not use the document's baseURI for our load events? Reading the spec, I think that's right, but would like to be sure before moving forward with the patches.

Comment 5

[James Graham [:jgraham]]

Yeah, the baseURI is used to resolve relative URLs, it's not the actual URL that will be loaded. Apologies if I missed this earlier.

Comment 6

[Julian Descottes [:jdescottes]]

Thanks James! Will move forward with the patches then.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/17440e444b7e
[bidi] Use document URL for load and domContentLoaded events r=webdriver-reviewers,Sasha
https://hg.mozilla.org/integration/autoland/rev/43d40f12fdf7
[wdspec] Add tests for bidi load events with base tags r=webdriver-reviewers,Sasha

Comment 8

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/39497 for changes under testing/web-platform/tests

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/17440e444b7e
https://hg.mozilla.org/mozilla-central/rev/43d40f12fdf7

Comment 10

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot