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

iOS pwa push notification issues #756

Open
huumn opened this issue Jan 16, 2024 · 10 comments
Open

iOS pwa push notification issues #756

huumn opened this issue Jan 16, 2024 · 10 comments

Comments

@huumn
Copy link
Member

huumn commented Jan 16, 2024

Figured we should just create persistent issue for this given they're numerous and hard to pin down:

  1. push notifications continue to be lost Push Notifications setting lost again #411
  2. notification reply counts and mention counts and subscribe to stacker counts are wacky
  3. when I click on notification replies I get sent to really old threads
  4. sat stacking notifications aren't replaced
@ekzyis ekzyis pinned this issue Jan 16, 2024
@ekzyis
Copy link
Member

ekzyis commented Jan 16, 2024

notification reply counts and mention counts and subscribe to stacker counts are wacky

I think in your case, this might be because of the race conditions (which don't happen on Android). If you receive a lot of notifications at the same time, the service worker returns stale data for getNotifications({ tag }) for some calls. I wanted to look into Web Locks API to workaround that.

  1. My deposit notifications are also weird now on Android. I got a "4 sats were deposited in your account" notification but only 1 sat was deposited [0].

I looked at the logs and they didn't fully explain what was going on. I noticed the bug that is fixed in #751 though.

I didn't change the Android code though. So I am confused. I also wasn't able to reproduce the issue locally. Seems to only affect deposit notifications.

[0] Created separate issue for this now: #763

@ekzyis
Copy link
Member

ekzyis commented Feb 18, 2024

Apple is pulling PWAs for their European users in iOS 17.4:

Why don't users in the EU have access to Home Screen web apps?

To comply with the Digital Markets Act, Apple has done an enormous amount of engineering work to add new functionality and capabilities for developers and users in the European Union — including more than 600 new APIs and a wide range of developer tools.

The iOS system has traditionally provided support for Home Screen web apps by building directly on WebKit and its security architecture. That integration means Home Screen web apps are managed to align with the security and privacy model for native apps on iOS, including isolation of storage and enforcement of system prompts to access privacy impacting capabilities on a per-site basis.

Without this type of isolation and enforcement, malicious web apps could read data from other web apps and recapture their permissions to gain access to a user’s camera, microphone or location without a user’s consent. Browsers also could install web apps on the system without a user’s awareness and consent. Addressing the complex security and privacy concerns associated with web apps using alternative browser engines would require building an entirely new integration architecture that does not currently exist in iOS and was not practical to undertake given the other demands of the DMA and the very low user adoption of Home Screen web apps. And so, to comply with the DMA’s requirements, we had to remove the Home Screen web apps feature in the EU.

EU users will be able to continue accessing websites directly from their Home Screen through a bookmark with minimal impact to their functionality. We expect this change to affect a small number of users. Still, we regret any impact this change — that was made as part of the work to comply with the DMA — may have on developers of Home Screen web apps and our users.

-- developer.apple.com, DMA and Apps in the EU, Developer Q&A

Even though this only should affect EU users ("a small number of users"), I guess this makes their lack of priority regarding this more clear.

So I believe we have come to a point where we seriously need to consider writing separate code just for Safari to use Safari Push Notifications:

If you’re already sending push notifications to users in Safari 15 or earlier using Safari Push Notifications, continue sending push notifications to those users. Update your webpage with feature detection to use Push API code if it’s available, or Safari Push Notifications code if it isn’t. For more information about best practices while sending a push notification with APNs, see Sending notification requests to APNs.

-- developer.apple.com, Sending web push notifications in web apps and browsers

I don't like this but I also want to get rid of this bug since it affects the ~20% of stackers that use iOS. I also have an iPhone now so I should be able to create an Apple developer account now (I think this is required) so I can implement this with no blockers.

@huumn
Copy link
Member Author

huumn commented Feb 18, 2024

Seems the way to go unless we want to roll out an iOS app and I'd guess their special push notifications are less buggy.

@huumn
Copy link
Member Author

huumn commented Jun 3, 2024

File

Someone sent me this when. They tried to turn on push notifications on iOS.

@ekzyis
Copy link
Member

ekzyis commented Jun 3, 2024

There is some service worker weirdness where it's not active on first render since it goes through some stages iirc, so refreshing the page should fix it.

But I'm also having problems on iOS enabling push notifications again after they were enabled before. Not sure if they turned off by themselves or if I turned them off. I currently get InvalidStateError: Push service initialization failed.

Push notifications on my Android phone still work, can turn them off and on with no problems

@ekzyis
Copy link
Member

ekzyis commented Sep 3, 2024

I think we lose the push subscription because the pushsubscriptionchange event has bad browser support (which was already known) but we still need to refresh the subscription regularly (which pushsubscriptionchange was meant for).

This article recommends to resubscribe on every notification:

When subscribing to push notifications, you often receive a PushSubscription.expirationTime of null. In theory, this means that the subscription never expires (in contrast to when you receive a DOMHighResTimeStamp, which tells you the exact point in time when the subscription expires). In practice, however, it's common for browsers to still let subscriptions expire, for example, if no push notifications were received for a longer time, or if the browser detects the user isn't using an app that has the push notifications permission. One pattern to prevent this is to resubscribe the user upon each received notification, as shown in the following snippet. This requires you to send notifications frequently enough for the browser to not have auto-expired the subscription, and you should very carefully weigh the advantages and disadvantages of legitimate notification needs against involuntarily spamming the user just so the subscription doesn't expire. In the end, you shouldn't try to fight the browser in its efforts to protect the user from long forgotten notification subscriptions.

We could try this with some checks that avoid too many resubscriptions.

@huumn
Copy link
Member Author

huumn commented Sep 3, 2024

This is definitely worth a revisit. My autowithdraw notifications now tell me > 1BTC is being withdrawn even when it's one sat.

@ekzyis
Copy link
Member

ekzyis commented Sep 3, 2024

I am not sure we can fix merging notifications for proper counts on iOS

@huumn
Copy link
Member Author

huumn commented Sep 3, 2024

I know it's unrelated to your comment. I just wanted to confirm that merging remains an issue.

@huumn
Copy link
Member Author

huumn commented Sep 4, 2024

Pretty nutty.

IMG_0785

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

No branches or pull requests

2 participants