Skip to content
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

Inconsistent browser detection #260

Closed
patricksnape opened this issue Feb 22, 2013 · 4 comments
Closed

Inconsistent browser detection #260

patricksnape opened this issue Feb 22, 2013 · 4 comments

Comments

@patricksnape
Copy link
Collaborator

I've noticed a variety of ways are used to detect the current browser. There seems to be browser detecting code that looks clean to me, but is barely used. Can we confirm that this code does indeed work? If it does, can we use it instead of using typeof(browser)? Opera's recent announcement to move to WebKit, and thus it's deprecation of the window.opera variable made me investigate our current method of browser detection. It looks like a lot of code will break in the next version of Opera, so shall we get consistent pro-actively?

If there is a widespread approval of this I will happily land a pull request for this.

@honestbleeps
Copy link
Owner

where have you noticed this exactly? besides needing to change the test for opera, typeof(chrome), safari etc I think are used pretty consistently. If not I'm up for having it fixed to be sure, but I'm curious where? Maybe in some code I folded in but didn't write?

@patricksnape
Copy link
Collaborator Author

L609, which admittedly is only once (and in Explorer code we don't currently support)... It's just submitting my delete cookies pull request I had to switch on the browser type and it felt pretty unclean. Maybe if we just add 4 methods to the browser detect that along the lines of

isChrome: { return typeof(chrome) !== undefined; },
isSafari: { return typeof(safari) !== undefined; },
isFirefox: { return typeof(self.on) === 'function'; },
isOpera: { return typeof(opera) !== undefined; }

At the moment, whenever browser decide to change how they identify themselves means sweeping all over the code for instances of safari, chrome etc. Would be nice to wrap it up in to some helper functions, no?

@honestbleeps
Copy link
Owner

ah gotcha, that one was added by the guy who made an attempt to get RES working in IE... it never fully got there.

agree a helper function would make maintenance easier!

patricksnape pushed a commit to patricksnape/Reddit-Enhancement-Suite that referenced this issue Feb 22, 2013
@patricksnape
Copy link
Collaborator Author

#258 now fixes this instead of two pull requests

honestbleeps added a commit that referenced this issue Mar 9, 2013
Fixes #257 and #260 - User account switcher broken
patricksnape pushed a commit to patricksnape/Reddit-Enhancement-Suite that referenced this issue Jun 23, 2013
patricksnape pushed a commit to patricksnape/Reddit-Enhancement-Suite that referenced this issue Jun 23, 2013
honestbleeps added a commit that referenced this issue Sep 2, 2013
Fixes #257 and #260 - User account switcher broken
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants