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

Review usage of webNavigation events with regard to pre-rendered documents #6665

Closed
fregante opened this issue Oct 13, 2023 · 2 comments
Closed
Labels
bug Something isn't working

Comments

@fregante
Copy link
Contributor

fregante commented Oct 13, 2023

https://developer.chrome.com/blog/extension-instantnav/#transitions

According to this document, onCommitted might fire twice on a single navigation:

  1. on a pre-rendered document
  2. on the visible document

We should make sure that we ignore documents are not yet visible.

As part of this issue, it might also be a good idea to see if the content scripts are injected in the pre-rendered state.

Triaging this as a bug because PB is definitely tracking pre-rendered documents right now, and it shouldn't

Copy link

This issue will be closed in 7 days unless the stale label is removed, or a comment is added to the issue.

@github-actions github-actions bot added the Stale label Feb 11, 2024
@fregante fregante removed the Stale label Feb 11, 2024
@fregante
Copy link
Contributor Author

This PR moved the onCommitted listener to the page itself:

Now we only use webNavigation listeners to keep track of the current tab's URL in the page editor in a hook, so it's fine if it fires twice with the same URL 🙌

const startWatching = once(async () => {
browser.webNavigation.onCommitted.addListener(onNavigation);
tabUrl = await getCurrentURL();
urlChanges.emit(tabUrl);
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

1 participant