-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add compatibility to Microsoft Edge #60
base: master
Are you sure you want to change the base?
Conversation
…patible with Edge (It's endpoint url is up to 500 characters long).
I'm so glad to see that - I have distressed on Microsoft Edge that can never receive the notifications for a long time. 👍 |
I tried to look up the description of the URL on Microsoft's website, but I got nothing but a statement that "URLs should be treated as black boxes"😟. So I checked the URLs posted in Devtool and compared them with the database, and finally found the problem. In current version it can be seen that the Endpoint URL stored in database is much shorter than in the post request the browser had sent.So that once Flarum tries to push notice then the webpush library will receive a 404 error and treat the URL as outdated. |
That's funny, when the debug mode is enabled, it told me that notifications have sent to my Edge🤣🤣🤣, maybe we had never consider this situation. By the way, this PR seems only changed the tables that from previous version - is that necessary to do more work to create a table with correct length for the new installations? |
All migrations run on new installs, so this should work fine once it's merged. |
Has this PR tested on MSEdge? |
Yes, with verion 126.0.2592.81 on my computer. |
Default string field with length 255 is too short for Microsoft Edge, which uses endpoint url up to 500 characters long.
Change length of field endpoint in table push_subscriptions to 1024.