Change way browser and version found #844

squirrelo · 2015-02-10T21:34:15Z

There was a problem with firefox and opera where the version found was not the actual version of the browser. This new code fixes that.

coveralls · 2015-02-10T21:56:36Z

Coverage remained the same at 78.48% when pulling 01adb2d on squirrelo:fix-firefox-validation into 8a791a3 on biocore:master.

ElDeveloper · 2015-02-10T22:04:39Z

qiita_pet/templates/sitebase.html

-          objfullVersion = ''+parseFloat(navigator.appVersion);
-          objBrMajorVersion = parseInt(navigator.appVersion,10);
-        }
+        // from http://stackoverflow.com/questions/5916900/how-can-you-detect-the-version-of-a-browser


Can you use the permanent link instead? http://stackoverflow.com/q/5916900/379593

ElDeveloper · 2015-02-10T22:10:04Z

This looks good except, there's not a good way to test this code, it has the "same" validity as the other chunk of code taken from a blog post found on the inter-tubes and that the code is not really maintainable. IMHO, should be ready to merge as soon as the comments I made are addressed but then again ... how can we make sure this new snippet is not introducing any problems that the previous block was accounting for? I would probably advocate for finding the bug in the previous block of code and correct it there.

antgonza · 2015-02-10T22:13:09Z

The issue is that there is no perfect way and more when browsers change the way they report their version, which made the previous one obsolete. I think this is fine as long as we test in all valid browsers.

ElDeveloper · 2015-02-10T22:18:45Z

Ok, sounds good. We should check Safari, Chrome, Opera, Internet
Explorer, Firefox and lynx? With the corresponding mobile versions where
possible?

On (Feb-10-15|14:13), Antonio Gonzalez wrote:

The issue is that there is no perfect way and more when browsers change the way they report their version, which made the previous one obsolete. I think this is fine as long as we test in all valid browsers.

Reply to this email directly or view it on GitHub:
#844 (comment)

antgonza · 2015-02-10T22:21:51Z

Up to you. I normally check Safari, Chrome, and Firefox.

ElDeveloper · 2015-02-10T23:58:49Z

Well the only browser that shows the error message is lynx, however the interface is unusable in internet explorer 8.

Just for fun:

wasade · 2015-02-11T20:39:09Z

@ElDeveloper, that is a fantastic screenshot

GabeAl · 2015-02-11T21:41:53Z

Still no good. I'm being inappropriately flagged (Firefox 35.0.1)

and cannot access the site. I also do not use Chrome and do not run a Mac. Firefox is the only truly open, cross-platform, non-corporate browser that enjoys modern standards support. For Linux (and Windows) users who can't (or won't) use Chrome, it's probably better to botch the detection and include an "unsupported browser" blurb at the top than to blanket ban.
Cheers

antgonza · 2015-02-11T21:43:46Z

Are you testing on this branch or on the main site?

GabeAl · 2015-02-11T22:06:18Z

Apologies -- actually, this is for the main site as it currently exists, not the proposed changes, thereby merely corroborating the finding in the opening message.

squirrelo · 2015-02-12T00:46:27Z

So apparently flake8 updated and is throwing all sorts of fun errors. I'll fix that in aseperate emergency pull request.

ElDeveloper · 2015-02-12T00:56:04Z

I'm on it, see #846

ElDeveloper · 2015-02-12T00:56:17Z

almost done also ...

squirrelo · 2015-02-12T01:00:41Z

Oops, pull request already in.

antgonza · 2015-02-12T15:20:36Z

Pull from upstream master.

…refox-validation

squirrelo · 2015-02-12T16:13:56Z

ready for review again.

antgonza · 2015-02-12T16:42:48Z

👍 @ElDeveloper ?

ElDeveloper · 2015-02-12T18:16:35Z

It still doesn't work in IE 8 i. e. no message about the website not working is shown. 😕

squirrelo · 2015-02-12T23:50:14Z

good to review again

ElDeveloper · 2015-02-13T00:02:20Z

It now shows the appropriate message for IE8, however it shows the
message in IE10 and Opera (it wasn't before), from the displayed message
it seems that at least IE10 is supported.

On (Feb-12-15|15:50), Joshua Shorenstein wrote:

good to review again

Reply to this email directly or view it on GitHub:
#844 (comment)

coveralls · 2015-02-13T00:05:26Z

Coverage decreased (-0.09%) to 78.5% when pulling 9eceade on squirrelo:fix-firefox-validation into 1ca033a on biocore:master.

squirrelo · 2015-02-13T00:10:15Z

Bluh, The issue is that the older browsers flat out won't run the version check javascript, so I defaulted it to just throw the error by default. Not sure whats going on with IE 10, but we don't support Opera so it's getting the default error page.

ElDeveloper · 2015-02-13T00:17:13Z

So then the only open issue is IE 10. And we may also need to rephrase
the error message to: "This site only works on these browsers".

On (Feb-12-15|16:10), Joshua Shorenstein wrote:

Bluh, The issue is that the older browsers flat out won't run the version check javascript, so I defaulted it to just throw the error by default. Not sure whats going on with IE 10, but we don't support Opera so it's getting the default error page.

Reply to this email directly or view it on GitHub:
#844 (comment)

squirrelo · 2015-02-13T00:18:52Z

Yup. IE isn't firing off the document.ready so I'll dig into that.

squirrelo · 2015-02-13T00:32:38Z

Went old school and used body onload='' to fire off the check, since that's supported by pretty much everything. This also means the site is visible by default now for non-checked browsers, so Opera will show the site again.

coveralls · 2015-02-13T00:50:18Z

Coverage decreased (-0.09%) to 78.5% when pulling 02e2ead on squirrelo:fix-firefox-validation into 1ca033a on biocore:master.

ElDeveloper · 2015-02-13T00:54:22Z

If we don't support Opera should that be changed then?

On (Feb-12-15|16:32), Joshua Shorenstein wrote:

Went old school and used body onload='' to fire off the check, since that's supported by pretty much everything. This also means the site is visible by default now for non-checked browsers, so Opera will show the site again.

Reply to this email directly or view it on GitHub:
#844 (comment)

squirrelo · 2015-02-13T04:14:36Z

I think we can have in the documentation that we support X browsers above Y version, and while the site is accessible with other browsers , we make no claims to usability.

antgonza · 2015-02-13T13:37:24Z

Agree, could you add that list to the help wiki?

antgonza · 2015-02-13T13:50:47Z

@ElDeveloper could you check again that it works on the expected browsers? Seems like you have everything in your system to do this ... thanks!

squirrelo · 2015-02-13T17:31:51Z

Added to the FAQ section.

antgonza · 2015-02-13T17:53:31Z

Thanks, looks fine: https://github.com/biocore/qiita/wiki/Frequently-Asked-Questions

ElDeveloper · 2015-02-13T18:43:12Z

qiita_pet/templates/sitebase.html

-      });
-    </script>
+      }
+


Extra whitespace.

ElDeveloper · 2015-02-13T18:48:38Z

Just tested this in all the browsers I have access to and it works just
fine! Lynx actually lets me log in (unlike IE8). Just one more minor
comment about whitespace and this should be merged right away!

On (Feb-13-15| 5:50), Antonio Gonzalez wrote:

@ElDeveloper could you check again that it works on the expected browsers? Seems like you have everything in your system to do this ... thanks!

Reply to this email directly or view it on GitHub:
#844 (comment)

squirrelo · 2015-02-13T19:04:45Z

whitespace removed, merge ready.

coveralls · 2015-02-13T19:21:10Z

Coverage decreased (-0.11%) to 78.48% when pulling d86181f on squirrelo:fix-firefox-validation into 1ca033a on biocore:master.

Change way browser and version found

change way browser and version found

01adb2d

ElDeveloper reviewed Feb 10, 2015
View reviewed changes

Fix indentation and add IE check

ebdb833

Merge branch 'master' of https://github.com/biocore/qiita into fix-fi…

5eaf3a1

…refox-validation

squirrelo added 2 commits February 12, 2015 15:43

default to showing overlay, turn off if proper version

a5a7c11

Move overlay div to bottom for text based browsers

9eceade

old-school onload check for browser compatability

02e2ead

ElDeveloper reviewed Feb 13, 2015
View reviewed changes

qiita_pet/templates/sitebase.html

});

</script>

}

Copy link

Contributor

ElDeveloper Feb 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra whitespace.

remove whitespace

d86181f

antgonza added a commit that referenced this pull request Feb 13, 2015

Merge pull request #844 from squirrelo/fix-firefox-validation

502774a

Change way browser and version found

antgonza merged commit 502774a into qiita-spots:master Feb 13, 2015

Change way browser and version found #844

Change way browser and version found #844

Uh oh!

Conversation

squirrelo commented Feb 10, 2015

Uh oh!

coveralls commented Feb 10, 2015

Uh oh!

ElDeveloper Feb 10, 2015

Choose a reason for hiding this comment

Uh oh!

ElDeveloper commented Feb 10, 2015

Uh oh!

antgonza commented Feb 10, 2015

Uh oh!

ElDeveloper commented Feb 10, 2015

Uh oh!

antgonza commented Feb 10, 2015 via email

Uh oh!

ElDeveloper commented Feb 10, 2015

Uh oh!

wasade commented Feb 11, 2015

Uh oh!

GabeAl commented Feb 11, 2015

Uh oh!

antgonza commented Feb 11, 2015 via email

Uh oh!

GabeAl commented Feb 11, 2015

Uh oh!

squirrelo commented Feb 12, 2015

Uh oh!

ElDeveloper commented Feb 12, 2015

Uh oh!

ElDeveloper commented Feb 12, 2015

Uh oh!

squirrelo commented Feb 12, 2015

Uh oh!

antgonza commented Feb 12, 2015

Uh oh!

squirrelo commented Feb 12, 2015

Uh oh!

antgonza commented Feb 12, 2015

Uh oh!

ElDeveloper commented Feb 12, 2015

Uh oh!

squirrelo commented Feb 12, 2015

Uh oh!

ElDeveloper commented Feb 13, 2015

Uh oh!

coveralls commented Feb 13, 2015

Uh oh!

squirrelo commented Feb 13, 2015

Uh oh!

ElDeveloper commented Feb 13, 2015

Uh oh!

squirrelo commented Feb 13, 2015

Uh oh!

squirrelo commented Feb 13, 2015

Uh oh!

coveralls commented Feb 13, 2015

Uh oh!

ElDeveloper commented Feb 13, 2015

Uh oh!

squirrelo commented Feb 13, 2015

Uh oh!

antgonza commented Feb 13, 2015 via email

Uh oh!

antgonza commented Feb 13, 2015

Uh oh!

squirrelo commented Feb 13, 2015

Uh oh!

antgonza commented Feb 13, 2015 via email

Uh oh!

ElDeveloper Feb 13, 2015

Choose a reason for hiding this comment

Uh oh!

ElDeveloper commented Feb 13, 2015

Uh oh!

squirrelo commented Feb 13, 2015

Uh oh!