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

Additional matching URLs for the notifications page #299

Conversation

scarf005
Copy link
Contributor

resolves #296

if (url.endsWith('/notifications')) {
matchingUrls.push(url + '?query=is%3Aunread');
if (new URL(url).pathname.endsWith('/notifications')) {
matchingUrls.push(url + '?query=*');
Copy link
Collaborator

Choose a reason for hiding this comment

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

The canonical link to the notifications page is https://github.com/notifications, not https://github.com/notifications?query= which you could be using here.

The code looks for any tabs that have /notifications or /notifications?query=is%3Aunread.

The actual fix here should be adding another matching URL to look for.

Suggested change
matchingUrls.push(url + '?query=*');
matchingUrls.push(url + '?query=');
matchingUrls.push(url + '?query=is%3Aunread');

This also does not require you to use the new URL(url).pathname, the param being passed to this function would always be a constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

applied in 1e30721 (#299).

scarf005 and others added 2 commits September 14, 2024 22:05
Co-authored-by: Laxman <notlmn@outlook.com>
@notlmn notlmn changed the title Allow any query for /notifications Additional matching URLs for the notifications page Sep 15, 2024
@notlmn notlmn merged commit 7aade10 into sindresorhus:main Sep 15, 2024
1 check passed
@notlmn
Copy link
Collaborator

notlmn commented Sep 15, 2024

Merged this, but beware that publishing to AMO is paused due to lack of full support of MV3. Further context in #298.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature request: more lax condition for reuse notification tabs when possible
2 participants