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

Chrome macOS 15 extra notification workaround #1209

Merged
merged 2 commits into from
Nov 19, 2024

Conversation

jkasten2
Copy link
Member

@jkasten2 jkasten2 commented Nov 18, 2024

Description

One Line Summary

Work around for extra "This site has been updated in the background" notification being shown on Chrome on macOS 15.

Details

macOS 15 made some unknown changes that causes Chrome's fragile notification display detection to fail, which means an extra notification with the body "This site has been updated in the background" displays.

This was reported to Google under this issue:
https://issues.chromium.org/issues/378103918

A workaround was found that if you delay ending the service worker event for a bit then Chrome will detect the notification from macOS. However this work around won't work 100% of the time since if a notification is closed very quickly (within the 1000ms). This window is also much easier to hit if notifications are sent back-to-back. Lastly it is unknown if 1000ms is always enough time, but too much time isn't good either due what was noted earlier.

We scoped this workaround to the known target of Chromium on macOS 15 so this ~1% chance doesn't cause issues on other platforms. We can't detect the macOS version so this will effect all versions.

What this does not fix:
There is also an issue where if the OneSignal feature received receipts / confirmed deliveries is on and the notification is clicked or closed with in it's wait window this extra notification will display. We will address this in a different PR.

Validation

Tests

Tested on macOS 15.1 with Chrome and Firefox.

Info

Checklist

  • All the automated tests pass or I explained why that is not possible
  • I have personally tested this on my machine or explained why that is not possible
  • I have included test coverage for these changes or explained why they are not needed

Programming Checklist
Interfaces:

  • Don't use default export
  • New interfaces are in model files

Functions:

  • Don't use default export
  • All function signatures have return types
  • Helpers should not access any data but rather be given the data to operate on.

Typescript:

  • No Typescript warnings
  • Avoid silencing null/undefined warnings with the exclamation point

Other:

  • Iteration: refrain from using elem of array syntax. Prefer forEach or use map
  • Avoid using global OneSignal accessor for context if possible. Instead, we can pass it to function/constructor so that we don't call OneSignal.context

Screenshots

Info

Checklist

  • I have included screenshots/recordings of the intended results or explained why they are not needed

Related Tickets



This change is Reviewable

macOS 15 made some unknown changes that causes Chrome's fragile
notification display detection to fail, which means an extra
notification with the body
"This site has been updated in the background" displays.

This was reported to Google under this issue:
https://issues.chromium.org/issues/378103918

A workaround was found that if you delay ending the service worker
event for a bit then Chrome will detect the notification from macOS.
However this work around won't work 100% of the time since if a
notification is closed very quickly (within the 1000ms). This window
is also much easier to hit if notifications are send back-to-back.
Lastly it is unknown if 1000ms is always enough time, but to much
time isn't good either due what was noted earlier.

We scoped this workaround to the known target of Chromium on macOS 15
so this ~1% chance doesn't cause issues on other platforms. We can't
detect the macOS version so this will effect all versions.

What this does not fix:
There is also an issue where if the OneSignal feature
received receipts / confirmed deliveries is on and the notification is
clicked or closed with in it's wait window this extra notification will
display. We will address this in a different PR.
Copy link
Contributor

@sherwinski sherwinski left a comment

Choose a reason for hiding this comment

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

Suggestion: If it's low-effort, I'd recommend adding a test. But without actual browser e2e tests it might not provide a lot of value.

@jkasten2
Copy link
Member Author

Suggestion: If it's low-effort, I'd recommend adding a test. But without actual browser e2e tests it might not provide a lot of value.

I added some tests in this commit
c3a3b09

Copy link
Contributor

@sherwinski sherwinski left a comment

Choose a reason for hiding this comment

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

Thanks for adding that 👍

@jkasten2 jkasten2 merged commit 6d3d603 into main Nov 19, 2024
4 checks passed
@jkasten2 jkasten2 deleted the workaround/macOS-Chrome-extra-notification branch November 19, 2024 19:47
@jkasten2 jkasten2 mentioned this pull request Nov 19, 2024
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.

2 participants