Skip to content

script: Merge Window::load_url with ScriptThread::navigate#43668

Merged
jdm merged 6 commits intoservo:mainfrom
thebabalola:script/merge-load-url-navigate
Mar 27, 2026
Merged

script: Merge Window::load_url with ScriptThread::navigate#43668
jdm merged 6 commits intoservo:mainfrom
thebabalola:script/merge-load-url-navigate

Conversation

@thebabalola
Copy link
Copy Markdown
Contributor

Both Window::load_url and ScriptThread::navigate implement parts of the same navigate algorithm from the HTML spec (https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigate). ScriptThread::navigate had only one caller: Window::load_url.

This merges both methods into a single navigate function in navigation.rs, which is the appropriate home for navigation logic. All previous callers of Window::load_url now call navigation::navigate directly.

Testing: This is a pure refactor with no behaviour changes, so no new tests are needed. Existing tests cover the navigation paths affected.
Fixes: #43494

@thebabalola thebabalola requested a review from gterzian as a code owner March 25, 2026 21:52
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 25, 2026
Copy link
Copy Markdown
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! Once it compiles, it's going to be a lot easier to see how to improve this code and make it better reflect the specification :)

Copy link
Copy Markdown
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Mar 26, 2026
@jdm jdm enabled auto-merge March 26, 2026 19:16
auto-merge was automatically disabled March 26, 2026 19:37

Head branch was pushed to by a user without write access

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 26, 2026
@jdm jdm enabled auto-merge March 26, 2026 22:07
auto-merge was automatically disabled March 26, 2026 23:11

Head branch was pushed to by a user without write access

@jdm jdm enabled auto-merge March 26, 2026 23:45
Both Window::load_url and ScriptThread::navigate implement parts of
the same navigate algorithm from the HTML spec
(https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigate).
ScriptThread::navigate had only one caller: Window::load_url.

This merges both methods into a single navigate function in
navigation.rs, which is the appropriate home for navigation logic.
All previous callers of Window::load_url now call navigation::navigate
directly.

Fixes: servo#43494
Signed-off-by: thebabalola <t.babalolajoseph@gmail.com>
Add missing WindowMethods trait import for window.Document()
and replace Trusted<GlobalScope> with Trusted<Window> to
eliminate the unnecessary is::<Window>() downcast check.

Signed-off-by: thebabalola <t.babalolajoseph@gmail.com>
…ript_url call

Signed-off-by: thebabalola <t.babalolajoseph@gmail.com>
Remove unused Trusted import from script_thread.rs and fix
needless borrow warnings in location.rs and windowproxy.rs.

Signed-off-by: thebabalola <t.babalolajoseph@gmail.com>
Signed-off-by: thebabalola <t.babalolajoseph@gmail.com>
Signed-off-by: thebabalola <t.babalolajoseph@gmail.com>
auto-merge was automatically disabled March 27, 2026 00:44

Head branch was pushed to by a user without write access

@thebabalola thebabalola force-pushed the script/merge-load-url-navigate branch from e034378 to 79b39aa Compare March 27, 2026 00:44
@thebabalola
Copy link
Copy Markdown
Contributor Author

@jdm .... Found the issue,on why the Ci keeps failing..... Servo uses extra unstable rustfmt options that I wasn't running locally. now gotten that Fixed. Could you approve the workflow one more time? This should be the last one, sorry about that

@jdm
Copy link
Copy Markdown
Member

jdm commented Mar 27, 2026

@thebabalola In future you can use ./mach fmt to avoid that problem!

@jdm jdm enabled auto-merge March 27, 2026 06:43
@jdm jdm added this pull request to the merge queue Mar 27, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Mar 27, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Mar 27, 2026
@servo-highfive servo-highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Mar 27, 2026
@jdm jdm added this pull request to the merge queue Mar 27, 2026
@servo-highfive servo-highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Mar 27, 2026
Merged via the queue into servo:main with commit ed8b28b Mar 27, 2026
36 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Mar 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Merge Window::load_url with ScriptThread::navigate

3 participants