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

browsingContext.navigationStarted for the initial navigation to about:blank #766

Open
sadym-chromium opened this issue Sep 4, 2024 · 10 comments
Labels
module-browsingContext Browsing Context module needs-discussion Issues to be discussed by the working group

Comments

@sadym-chromium
Copy link
Contributor

Analogous to the initial browsingContext.load event, it can be confusing, especially if the browsing context is created, navigated to about:blank and then navigated to some other URL.

From discussion:

Hmm, so about:blank is tricky, and gecko are trying to change their behaviour so that the load is synchronous like Blink and WebKit. Given the current HTML spec, it's not clear if you can expect to observe a load event for the initial about:blank, so I think it makes some amount of sense to not have our tests depend on this tricky area of interop. I think for now it would be OK to just ignore any load event if the URL is about:blank, which I think would avoid the possible race without adding in yet another load.

Do we need those events for the initial navigation? If not, should we specify an exception?

@sadym-chromium
Copy link
Contributor Author

We can mark those tests as tentative for now: web-platform-tests/wpt#47958

@whimboo
Copy link
Contributor

whimboo commented Sep 4, 2024

We cannot simply ignore all the navigations to about:blank (as proposed above) given that such a navigation can happen at any time by a test as well, which then has nothing to do with the initial navigation when the tab gets created.

But yes, we should discuss and maybe you can add it to the next meeting which probably will be the TPAC one this month?

@whimboo whimboo added the module-browsingContext Browsing Context module label Sep 4, 2024
@sadym-chromium
Copy link
Contributor Author

sadym-chromium commented Sep 4, 2024

We cannot simply ignore all the navigations to about:blank (as proposed above)

Let me clarify: about:blank can be a valid navigation, and I don't propose ignoring all the about:blank navigations. This issue is regarding only the initial navigation when the browsing context is created.

@jgraham
Copy link
Member

jgraham commented Sep 5, 2024

I think in principle I'm happy with the idea of trying to ignore the navigation to initial about:blank so that from the user point of view it's an atomic part of the navigable being created. So no events relating to the load of that document, maybe also delaying the context created event until after it's loaded, and preventing any commands running in a context while any initial about:blank has not completed loading.

However the technical here are tricky, and I haven't looked in great detail.

@OrKoN
Copy link
Contributor

OrKoN commented Sep 5, 2024

Does the spec actually go through the navigation started hook when creating a new browsing context? I do not seem to be able to confirm that.

@whimboo
Copy link
Contributor

whimboo commented Sep 13, 2024

Note that this issue also affects the browsingContext.contextCreated event. If emitted too early, clients may assume the tab is ready for interaction and begin sending commands, leading to potential errors like this one.

Which events should we actually emit, and when, during the browsingContext.create command? Here's a proposal:

  • Avoid sending navigation and network events. These events shouldn't be triggered prematurely, especially during the initial load of newly created tabs.
  • Delay events like browsingContext.contextCreated, script.realmCreated, and potentially others until the initial load is complete.

Is there anything else we should consider to ensure a smooth and error-free interaction? What are your thoughts on this approach?

@css-meeting-bot
Copy link
Member

The Browser Testing and Tools Working Group just discussed Emitting Events During "browsingContext.create" when "about:blank" is loaded, and agreed to the following:

  • ACTION: sadym to update test to hsow that events are not emited on about:blank
The full IRC log of that discussion <AutomatedTester> topic: Emitting Events During "browsingContext.create" when "about:blank" is loaded
<AutomatedTester> github: https://github.com//issues/766
<AutomatedTester> scribe+
<AutomatedTester> whimboo: this issue was origianly raised by sadym
<AutomatedTester> ... this is about getting an event when the context being created and then when can we navigate
<orkon> q+
<AutomatedTester> ... so the question is what do we want to do with the creation of new tabs/documents?
<AutomatedTester> ... do we want to handle navigationStarted for about:black?
<AutomatedTester> ack orkon
<AutomatedTester> orkon: I checked the html spec and I couldn't see if there is a navigationStarted with about:blank
<AutomatedTester> ... perhaps we need to reverse the test so it doesn't have the event on about:blank
<AutomatedTester> q?
<whimboo> q+
<AutomatedTester> ack whimboo
<AutomatedTester> whimboo: we are happy to change this
<AutomatedTester> ... and this way we can improve the clients so they have a better idea when to work with the tab
<jdescottes> q+
<AutomatedTester> ... and I am not sure about the implementation status of these things in chrome and if/when it will be ready
<AutomatedTester> ack jdescottes
<AutomatedTester> jdescottes: when I was testing this in Firefox we weren't running events for navigation on about:blank
<orkon> q+
<AutomatedTester> ... if there are any scenario where we have any extra events in the browser?
<AutomatedTester> ack orkon
<orkon> https://github.com/web-platform-tests/wpt/pull/47958/files#diff-c773a239b93f48290e8f834f6fd9c337720fb92b206b65e03ce1c64633789032
<jdescottes> q+
<AutomatedTester> whimboo: we fire the created event early... this is a combined question of should we send anything out before the creation of the tab
<AutomatedTester> orkon: there are wpt tests that I have linked
<AutomatedTester> ... [describes test]. This test wasnt passing in chrome but in Firefox
<AutomatedTester> q?
<AutomatedTester> ack jdescottes
<AutomatedTester> jdescottes: thanks to the link. THe item about the creation event coming out too early... I think we should talk about this.
<gsnedders> q?
<sadym> This is the mentioned test: https://github.com/web-platform-tests/wpt/pull/47958
<AutomatedTester> ... in firefox we have changed the way that we emit events so that we do it after the creation of the context
<AutomatedTester> ack gsnedders
<orkon> q+
<AutomatedTester> gsnedders: Are you saying the load is async in firefox? I thought you could always access the document in the iframe in a sync way
<AutomatedTester> whimboo: this is for the new top level browser context
<AutomatedTester> gsnedders: ok, that wasn't clear in the issue
<AutomatedTester> ack orkon
<AutomatedTester> orkon: looks like we're talking about 2 different issues. This is about the browserContext creation with about:blank
<sadym> https://github.com/web-platform-tests/wpt/pull/47958/files#diff-c773a239b93f48290e8f834f6fd9c337720fb92b206b65e03ce1c64633789032R14-R29
<AutomatedTester> ... in firefox there are navigationStarted events being fired in about;blank
<AutomatedTester> sadym: I've linked to the the test
<AutomatedTester> q?
<AutomatedTester> jgraham: for the case of initial about:blank we should treat it like it is sync
<AutomatedTester> ... e.g. create a new tab with about:blank you can start interacting with it straight away
<jgraham> jgraham: and not emit a navigation started event
<AutomatedTester> gsnedders: is there a use case where that is not about:blank?
<AutomatedTester> [discussion about checking we are all started from about:blank and navigationStarted events]
<AutomatedTester> whimboo: firefox has 2 events emited for about:blank. a sync and an async event
<orkon> q+
<simonstewart> q?
<AutomatedTester> ... and we should treat new window creation special if there is a URL in there
<AutomatedTester> whimboo: how would window creation handle this?
<AutomatedTester> ack orkon
<AutomatedTester> orkon: the html spec says there are some special treatment for about blank. e.g. if it has params we need to update the history
<AutomatedTester> whimboo: this should be possible for us to do this and not emit the event
<AutomatedTester> q?
<AutomatedTester> ACTION: sadym to update test to hsow that events are not emited on about:blank

@whimboo
Copy link
Contributor

whimboo commented Oct 15, 2024

Note that we are going to update the behavior of Firefox over on https://bugzilla.mozilla.org/show_bug.cgi?id=1922014.

@whimboo
Copy link
Contributor

whimboo commented Oct 15, 2024

@sadym-chromium can you please update the related tests so that they actually behave as expected? Thanks.

@sadym-chromium
Copy link
Contributor Author

@sadym-chromium can you please update the related tests so that they actually behave as expected? Thanks.

I started working on it in web-platform-tests/wpt#48651

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module-browsingContext Browsing Context module needs-discussion Issues to be discussed by the working group
Projects
None yet
Development

No branches or pull requests

5 participants