-
Notifications
You must be signed in to change notification settings - Fork 419
Don't check typeof browser to detect Firefox
#8684
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
Conversation
|
what if chrome goes and adds this api too though, also maybe try more strict checking? stuff like |
|
My first idea was to call the
That's probably better. We could also just check the |
yeah that is what I do when I am to bored to get feature based detection, I do the error thing and the userAgent. (though the user agent lies sometimes) |
|
I've changed the detection code so that it works outside the extension context. I used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ik my review means nothing but, this works on older chromium versions and firefox too.
The window.browser namespace will soon exist in both Chromium and Firefox.
Resolves #8682
Changes
Changes the way background and settings page code detects that it's running on Firefox: instead of checking if
window.browserexists, it now checks if a Firefox-specific API is available. I've made a shared module to detect the browser so that it's easy to update in the future if needed.Reason for changes
The
window.browsernamespace will soon exist in both Chromium and Firefox.Tests
Tested on Edge v144 (dev channel) and Firefox.