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

chore: refactor navigation #2881

Merged
merged 22 commits into from
Dec 16, 2024
Merged

chore: refactor navigation #2881

merged 22 commits into from
Dec 16, 2024

Conversation

sadym-chromium
Copy link
Collaborator

@sadym-chromium sadym-chromium commented Dec 10, 2024

Addressing Align navigation with the spec.

Navigation tracker relies on the following events:

  • Page.frameNavigated
  • TargetEvents.FrameStartedNavigating (Network.requestWillBeSent)
  • Page.navigatedWithinDocument
  • Page.frameRequestedNavigation
  • Page.javascriptDialogOpening:beforeunload

BEGIN_COMMIT_OVERRIDE
fix!: align navigation started with the spec
END_COMMIT_OVERRIDE

@sadym-chromium sadym-chromium added the update-expectations Update WPT expectations label Dec 10, 2024
@sadym-chromium sadym-chromium force-pushed the sadym/refactor-navigation-4 branch 2 times, most recently from 6b71d29 to 7f50236 Compare December 11, 2024 13:14
@sadym-chromium sadym-chromium added the puppeteer Run Puppeteer test when added to PR label Dec 11, 2024
@sadym-chromium sadym-chromium added the DO NOT MERGE Don't merge the marked PR label Dec 11, 2024
@sadym-chromium sadym-chromium changed the title [EXPERIMENT] Align navigation with the spec Dec 11, 2024
@sadym-chromium sadym-chromium changed the title Align navigation with the spec [DRAFT] Align navigation with the spec Dec 11, 2024
@sadym-chromium sadym-chromium requested a review from OrKoN December 11, 2024 16:25
@sadym-chromium sadym-chromium removed the DO NOT MERGE Don't merge the marked PR label Dec 11, 2024
@sadym-chromium sadym-chromium changed the title [DRAFT] Align navigation with the spec chore: refactor navigation Dec 11, 2024
@OrKoN
Copy link
Collaborator

OrKoN commented Dec 11, 2024

There are a few new Puppeteer failures as well as far as I can see.

@sadym-chromium sadym-chromium force-pushed the sadym/refactor-navigation-4 branch 2 times, most recently from ba31234 to b506b68 Compare December 12, 2024 15:46
@OrKoN
Copy link
Collaborator

OrKoN commented Dec 12, 2024

The following Puppeteer tests are new regressions with this PR:

  • "[navigation.spec] navigation Page.goto should work when reload causes history API in beforeunload" FAIL
  • "[navigation.spec] navigation Page.goto should work when navigating to data url" FAIL
  • "[navigation.spec] navigation Page.goto should navigate to dataURL and fire dataURL requests" FAIL
  • "[requestinterception-experimental.spec] cooperative request interception Page.setRequestInterception should navigate to dataURL and fire dataURL requests" FAIL
  • "[requestinterception.spec] request interception Page.setRequestInterception should navigate to dataURL and fire dataURL requests" FAIL
  • "[oopif.spec] OOPIF should load oopif iframes with subresources and request interception" FAIL

@OrKoN OrKoN force-pushed the sadym/refactor-navigation-4 branch from 752d25a to 8e71934 Compare December 12, 2024 21:36
@OrKoN
Copy link
Collaborator

OrKoN commented Dec 12, 2024

I have landed the roll with existing changes into Puppeteer so that new regressions are clear. If anything fails in the Puppeteer job now, it is caused by the changes in this PR.

@OrKoN
Copy link
Collaborator

OrKoN commented Dec 13, 2024

I have landed the roll with existing changes into Puppeteer so that new regressions are clear. If anything fails in the Puppeteer job now, it is caused by the changes in this PR.

I'd suggest we add e2e tests/fixes for Puppeteer scenarios that this change affects.

@OrKoN
Copy link
Collaborator

OrKoN commented Dec 13, 2024

"[oopif.spec] OOPIF should load oopif iframes with subresources and request interception" seems to be a Linux-only failure upstream.

@OrKoN OrKoN force-pushed the sadym/refactor-navigation-4 branch from 2bb265e to 14f5df6 Compare December 13, 2024 10:26
@sadym-chromium sadym-chromium force-pushed the sadym/refactor-navigation-4 branch 2 times, most recently from a9bb606 to c47a4e6 Compare December 13, 2024 15:34
@sadym-chromium sadym-chromium force-pushed the sadym/refactor-navigation-4 branch from c47a4e6 to 0bf727c Compare December 13, 2024 15:39
@sadym-chromium sadym-chromium marked this pull request as ready for review December 16, 2024 09:53
@sadym-chromium sadym-chromium merged commit 960531f into main Dec 16, 2024
52 of 53 checks passed
@sadym-chromium sadym-chromium deleted the sadym/refactor-navigation-4 branch December 16, 2024 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
puppeteer Run Puppeteer test when added to PR update-expectations Update WPT expectations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants