search

jazzband / django-push-notifications

Send push notifications to mobile devices through GCM or APNS in Django.
MIT License
2.24k stars 605 forks source link

Incompatibility with userAgentData. #640

Closed dismine closed 10 months ago

dismine commented 2 years ago

Hi.

I used loadVersionBrowser function from the documentation. And my view stopped working without producing any errors in logs. Later I figured out that it was caused by unexpected browser name. I use Vivaldi and userAgentData recognized it as Chromium, not Chrome as we expect, while old user agent string still claims that we are Chrome.

simonkern commented 2 years ago

The linked PR could also solve your issue. Could you try it?

dismine commented 2 years ago

Hi.

I have tested your code. For Vivaldi the situation is the same. navigator.userAgentData returns Chromium. It correctly returns Chrome only when fallback to parsing a user agent string. In my case it is Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/96.0.4664.113 Safari/537.36.

So, does it solve my issue? Kind of, yes. But why modern way to get a browser brand doesn't work with Chromium based browsers?

simonkern commented 2 years ago

if (brand.match(/opera|chrome|edge|safari|firefox|msie|trident/i) !== null)

This condition can never be true for the string "Chromium", which is why it should fallback to userAgent anyway. Are you sure the function returns "Chromium" as browsername?

"Chromium".match(/opera|chrome|edge|safari|firefox|msie|trident/i)
null

Vivaldi wouldn't be in the regexp anyway. Why they do not supply their browsername, like Edge does - I have no idea...

dismine commented 2 years ago

Are you sure the function returns "Chromium" as browsername?

Your function returns Chrome. So, yes, it works correctly.

Vivaldi wouldn't be in the regexp anyway. Why they do not supply their browsername, like Edge does - I have no idea...

I can only guess.

jamaalscarlett commented 2 years ago

@dismine https://github.com/jazzband/django-push-notifications/pull/643 seems to have fixed your issue, can we close this?