Created attachment 8197
Bug fixed?

Moment of boredom...

Bare in mind, I don't usually do Javascript ;)

Seems to work roughly in the console...

attachment bug 27644.patch ignored as obsolete

(In reply to comment #1)

Created attachment 8197 [details]
Bug fixed?

Moment of boredom...

Bare in mind, I don't usually do Javascript ;)

Seems to work roughly in the console...

I'm fairly sure the same kind of check won't work for all of them. It works for IE6 but that's probably the exception:

IE6:
< navigator.appVersion

"4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1)"

< navigator.appVersion.indexOf( 'MSIE' )

17

< navigator.appVersion.split( 'MSIE' )

["4.0 (compatible; ", " 6.0; Windows NT 5.1; SV1)"]

< navigator.appVersion.split( 'MSIE' )[1]

" 6.0; Windows NT 5.1; SV1)"

< parseFloat(navigator.appVersion.split( 'MSIE' )[1])

6

But that won't work for Firefox which for example has it's version at the end of the useragent string seperated by a slash

ie.:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.7) Gecko/20070918 Firefox/2
or:
< navigator.appVersion

"5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.13) Gecko/20100914 Firefox/3.5.13"

< navigator.appVersion.split('Firefox')

["5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.13) Gecko/20100914 ", "/3.5.13"]

< navigator.appVersion.split('Firefox')[1]

/3.5.13

< parseFloat(navigator.appVersion.split('Firefox')[1])

NaN

attachment bug 27644.patch ignored as obsolete

We'll need different manual checks for different browsers.

A few more examples of the complete navigator.userAgent (navigator.appVersion is a substring of that).

!!! DON'T RELY ON THESE! they're not from the older versions which we need to test for

• Opera 10.62 Windows (notice how the release version is at the end. Not 9.80)

"Opera/9.80 (Windows NT 6.1; U; en) Presto/2.6.30 Version/10.62"

• (Mobile) Safari 3.2.2 (iPad)

"Mozilla/5.0 (iPad; U; CPU OS 3_2_2 like Mac OS X; en-us) AppleWebKit/531.21.10 (KHTML, like Gecko) Version/4.0.4 Mobile/7B500 Safari/531.21.10"

• Safari 5.0.3 (Windows) (notice how the release version is not with the application name; "5.0.3")

"Mozilla/5.0 (Windows; U; Windows NT 6.0; en-us) AppleWebKit/533.19.4 (KHTML, like Gecko) Version/5.0.3 Safari/533.19.4"

(In reply to comment #2)

I'm fairly sure the same kind of check won't work for all of them. It works for
IE6 but that's probably the exception:

Why am I not suprised?

navigator.appVersion
"5.0 (X11; U; Linux x86_64; en-US) AppleWebKit/534.20 (KHTML, like Gecko) Chrome/11.0.672.2 Safari/534.20"
navigator.appVersion.split('Chrome')
["5.0 (X11; U; Linux x86_64; en-US) AppleWebKit/534.20 (KHTML, like Gecko) ", "/11.0.672.2 Safari/534.20"]
navigator.appVersion.split('Chrome')[1]
"/11.0.672.2 Safari/534.20"

I guess that possibly means Safari gets a false positive on Chrome, though the "version number" is much higher

What also wouldn't suprise me, if there were some browsers that have changed their useragent formatting over the years...

navigator.appVersion.split('Chrome/')[1]
"11.0.672.2 Safari/534.20"
parseFloat(navigator.appVersion.split('Chrome/')[1])
11

That works...

So, that's 3 we can do with the same Format - FF, Chrome, IE

N-2 solutions is better than N solutions (where N is the number of browsers)

Possibly Opera and Safari as one (bar iPad)

+ return !( browserVersionCheck( 'MSIE', 6 )
+ || browserVersionCheck( 'Chrome/', 1 )
+ || browserVersionCheck( 'Firefox/', 2 )
+ || browserVersionCheck( 'Safari', 3 )
+ || browserVersionCheck( 'Opera', 9 )
+ );

Should work then....

(In reply to comment #6)

+ return !( browserVersionCheck( 'MSIE', 6 )
+ || browserVersionCheck( 'Chrome/', 1 )
+ || browserVersionCheck( 'Firefox/', 2 )
+ || browserVersionCheck( 'Safari', 3 )
+ || browserVersionCheck( 'Opera', 9 )
+ );

Should work then....

No because Opera also uses a slash, and, moreover, the release version in opera browsers is after "Version/" not after "Opera/" (atleast in the current Opera).

Same thing with Safari " (... ) Version/5.0.3 Safari/533.19.4 (..) "

The Safari release version is 5.0.3, "533.19.4" is an internal code (presumably a build number from Apple and/or of the WebKit engine specifically)

"Firefox/" would work I guess (including the slash in the split)

Chrome is tricky indeed. Apple released Safari but they're also behind WebKit (although WebKit is open source, it's still backed by Apple in some ways) - so the "Safari"-string gets in the face from time to time when using non-Safari WebKit browsers.

See also the source of http://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/resources/jquery/jquery.client.js?view=markup

and look at:
http://ajax.googleapis.com/ajax/libs/jquery/1.5.0/jquery.js
It has some interesting stuff. Seperated over three sections (ctrl-f find them):
// Useragent RegExp
and
uaMatch: function( ua ) {
and
browserMatch.browser

I was meaning for the first 3. I submitted it, and then realised i hadn't commented to say that.

Checking for Chrome before Safari in the list helps reduce Safari false positives

One would almost wonder, why jQuery itself doesn't have a simple javascript function to do exactly that

doesBrowserSupportjQuery()

Something like http://api.jquery.com/jQuery.support/ perhaps?

(In reply to comment #10)

Something like http://api.jquery.com/jQuery.support/ perhaps?

No, not really. For one, we can't use jQuery since we're testing to detect browsers that don't support jQuery, jQuery itself isn't loaded yet. The startUp module is the very first thing that's executed.

Secondly, although we could use parts of jQuery's source code, jQuery.support is an object to help with feature detection, not browser detection.

(In reply to comment #9)

One would almost wonder, why jQuery itself doesn't have a simple javascript
function to do exactly that

doesBrowserSupportjQuery()

Good point, I think I read an article from Paul Irish (jQuery developer) on this a few weeks ago. I'll ask him about it.

Seems nicer than reinventing the wheel seemingly unnecessarily

Okay, after some chat on IRC I'm going to mark this WONTFIX.

IE6 isn't an issue since we actually support that (and so does jQuery, for now).

The numbers in the comparison may be confusing but it's about "less than", so what were planning to blacklist is:

• IE 5.5 and below
• Firefox 1.5 and below
• Safari 2 and below
• Opera 8 and below

According to http://stats.wikimedia.org/wikimedia/squids/SquidReportClients.htm those are way to small to actually spend time on in core.

jQuery was loaded on all pages in 1.16wmf fine without any known bugs or complaints, and unless needed we're not starting.

This might be a good place to think about mobile redirects too, if we are indeed moving it to the top especially.

(In reply to comment #14)

This might be a good place to think about mobile redirects too, if we are
indeed moving it to the top especially.

Good point. Once startup lives at the top, the mobile redirect should totally be in there.

However, the mobile redirect script currently uses a few MW-exported JS vars (wgPagename etc.), we'd have to export those early. Not too hard if we write a special module for it.

(In reply to comment #15)

Good point. Once startup lives at the top, the mobile redirect should totally
be in there.

However, the mobile redirect script currently uses a few MW-exported JS vars
(wgPagename etc.), we'd have to export those early. Not too hard if we write a
special module for it.

Gah. Can't we just kill the JS redirect altogether? I thought it was a temporary stop-gap measure before server-side support was implemented. I think any efforts should be focused on killing it altogether, not trying to find ways to make it work with ResourceLoader.

(In reply to comment #16)

Gah. Can't we just kill the JS redirect altogether? I thought it was a
temporary stop-gap measure before server-side support was implemented. I think
any efforts should be focused on killing it altogether, not trying to find ways
to make it work with ResourceLoader.

That's all very true, but the fact of the matter is that the group of people with the skills to implement the mobile redirect server side (which, AFAIK, is blocking on migrating from Squid to Varnish) is completely separate from the group of people with the skills to make the JS redirect suck less. The JS redirect is a stopgap measure, but that doesn't mean it's not warranted for a JS dev to spend an hour making it better while the ops folks figure out how to do this server-side.