servo / servo/servo

Fix window.open loading

Open
#31,982 5 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

A-content/script
Dominant language
Rust
Stars
38k
Forks
3.8k
Avg merge
1d 6h
Merged PRs (30d)
465

Description

This is similar to https://github.com/servo/servo/issues/31973

When doing window.open the about:blank case should not result in any navigation and firing of load events.

Spec: https://html.spec.whatwg.org/multipage/#window-open-steps

In our current implementation, we do:

  1. A synchronous load of a new BC. This always results in firing a load event.
  2. We start a navigation-like workflow via ScriptMsg::ScriptNewAuxiliary

So, specwise, the about:blank case needs to be handled through the step at https://html.spec.whatwg.org/multipage/nav-history-apis.html#apis-for-creating-and-navigating-browsing-contexts-by-name:url-and-history-update-steps, which prevents the "otherwise, navigate" step below it.

In terms of fixing the implementation, I would say for "about:blank" we should create the "navigable"(browsing context sort of), but not do any navigation and not fire any load events. Specifically, the fact that ScriptThread::process_attach_layout always results in firing load events is wrong, and so is the fact that ScriptMsg::ScriptNewAuxiliary always results in a kind of navigation(I think). We would still need to somehow "tell" the constellation of the newly created BC, in case it is navigated later.

The relevant test is at /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-window-open.html. For some reason prior to https://github.com/servo/servo/pull/31505 we would pass one subtest, but since the test asserts unreached before a 50ms timeout, I think it was just luck(although not intermittent).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in components/script/dom/windowproxy.rs around the ScriptMsg::ScriptNewAuxiliary call and synchronous new browsing-context load. Trace ScriptThread::process_attach_layout and the constellation handoff, then run html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-window-open.html. Done means about:blank creates the navigable without navigation or load events, while allowing later navigation.

Written by the indexing model from the issue text.

Assessment

Tech stack
html, rust
Domain
testing-qa, web-dev
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.