Is Safari intentionally excluded from the Service Worker installation?

Probably safari didn't support service workers when this code was added.

> @baris said:
>
> Probably safari didn't support service workers when this code was added.

So, would it be possible to revert Safari to a standard Service Worker installation today?
It may still only support it when the page is installed as a PWA, so you need to implement exception handling by checking something like this:

if (!('serviceWorker' in navigator)) {
    return;
}

@baris As an iPhone user with Safari, I would really like to see this fixed.
@AMAARETS' solution seems great to me, and I would be happy if they would fix it officially.

If someone with safari can confirm it works when !config.useragent.isSafari is removed, we can make that change.

> @baris said:
>
> If someone with safari can confirm it works when !config.useragent.isSafari is removed, we can make that change.

I tested it, and indeed it helped.. so you can make this change..
Only there is a very annoying bug

If my friend sends me three private messages in a row, whose content is "Message 1" "Message 2" "Message 3" I would receive three notifications with the following content: "Message 1" "Message 1" "Message 1"
Which means I would receive the same content of the first message in the three notifications...

Did you understand the problem and can you fix it?

It seems to me that this happens because when sending several messages in a row, the message ID is not the same, and then it takes the content from the first message.

I think that might be just the notification merging that happens in core, not something related to service worker.

> @baris said:
>
> I think that might be just the notification merging that happens in core, not something related to service worker.

So can you fix this please?

@שניאור-שמח can you share a screenshot of the issue you are seeing, is it the notification in nodebb or the email notification?

@baris Here you can see that in all notifications you see the same content of message number 1.

These are push notifications so they are likely handled in the web-push plugin, I see a PR there https://github.com/NodeBB/nodebb-plugin-web-push/pull/64 that might fix this issue @julian

@baris That's right, that's what I have installed, but it only helped the notifications appear on the iPhone in the Safari browser, but it still didn't help with the notification merging problem.

So I ask you to try to sort this out.
thanks.

@שניאור-שמח are chats the only type of notification failing at merging?

@baris web push is a direct reflection of regular notifications so if they share merge id, the old push notif is removed and replaced

@julian seems like safari is picky about replacing same notifs with tag, the PR on the repo has some code to handle safari via mergeId.

> @julian said:
>
> are chats the only type of notification failing at merging?

Yes, the problem is only with private messages, where several messages are sent in a row.

> @baris said:
>
> the PR on the repo has some code to handle safari via mergeId.
>
>

My friend @AMAARETS is the one who did what I asked him to do.
But it still didn't help the problem.

The code in that pull request isn't merged yet, so it is not active in the latest version of nodebb. Once it's merged and published you can install and test it. Or you can replace the plugin with your own version for testing.

@baris This is what I did, I installed the plugin with the changes and it still didn't help..
What I'm trying to explain is that the change my friend made for message merging didn't help the problem..
So I would appreciate it if you could see what the problem is and fix it..
Thank you

@שניאור-שמח unfortunately I don't have an iphone so I can't test the changes.

@baris I have an iPhone, I'll check what's needed, but you guys do what's needed to fix the bug and I'll check if it worked or not..
I just don't understand the system very well, and I wasn't able to fix the problem on my own..