Add compatibility with most browsers supporting WebExtension #1
Conversation
* Use browser-polyfill.js (https://github.com/mozilla/webextension-polyfill) * For auth, send extension URL instead of extension ID to support schemes like `moz-extension://` * Fix manifest errors (wrong logo sizes for 16px and 48px) * Optimize PNGs using `pngquant` * Remove `.DS_Store` files * Some code style fixes applied by prettier (I have format on save enabled and I noticed too late)
|
Hi @alin23 Much needed patch. Thank you for taking the time to contribute back. Best, |
|
To help with the review this is an outline of the code changes:
|
| passport.authenticate('google', { failureRedirect: '/login' }), | ||
| function(req, res) { | ||
| const redirect = req.session.returnTo; | ||
| delete req.session.returnTo; | ||
|
|
||
| if (req.session.from_extension) { |
There was a problem hiding this comment.
This parameter name change from_extension to from_extension_url might break the app if the server update and extension update doesn't happen at the same time – which is hard to ensure, given that the extension update is in the browser's control.
Can you edit the patch to exclude these param change alone?
There was a problem hiding this comment.
Since the server handles both cases, would it be hard to ensure the server is updated first?
I think you could deploy the server first, which would still be compatible with the current version of the extension, and then you can publish the extension on the web store.
What do you think?
There was a problem hiding this comment.
Ah, makes sense. I didn't notice you've preserved the old handling as well. That should do 👍
|
Neat & timely! Merged now. |
moz-extension://pngquant.DS_StorefilesTested on latest Chrome Canary and Firefox Developer Edition