Opened 12 years ago

Closed 11 years ago

#6744 closed defect (fixed)

dojo.isSafari doesn't work properly for Safari 2.0.4

Reported by: jgarfield Owned by: Adam Peller
Priority: high Milestone: 1.2
Component: Core Version: 1.1.0
Keywords: dojo, isSafari, dojo.isSafari, core, browser sniff Cc:
Blocked By: Blocking:

Description (last modified by Adam Peller)

The browser sniffing in the hostenv_browser.js needs to have the parsing for WebKit? 419.3 (Safari 2) changed from

parseFloat(dav.substr(idx+7)) >= 419.3

to

parseFloat(dav.substr(idx+8)) >= 419.3

Otherwise you end up with 3 as the Safari version, even though you're using Safari 2.0.4.



Safari 2.0.4 Strings

AppVersion - 5.0 (Macintosh; U; PPC Mac OS X; en) AppleWebKit/418.8 (KHTML, like Gecko) Safari/419.3

UserAgent - Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en) AppleWebKit/418.8 (KHTML, like Gecko) Safari/419.3

Change History (9)

comment:1 Changed 12 years ago by Adam Peller

Owner: changed from anonymous to alex

comment:2 Changed 12 years ago by Adam Peller

Description: modified (diff)

comment:3 Changed 12 years ago by alex

Status: newassigned

comment:4 Changed 11 years ago by dylan

Milestone: 1.2

This currently breaks proper Safari browser detection... should be fixed for 1.2 in my opinion.

comment:5 Changed 11 years ago by Adam Peller

Owner: changed from alex to Adam Peller
Status: assignednew

code looks right to me. asking jgarfield for clarification.

comment:6 Changed 11 years ago by jgarfield

I finally got hold of our Mac at work again, and here is what I found in regard to this issue...

The issue isn't actually related to the addition of 7 or 8 to the idx value, but rather the condition following it...

d.isSafari = parseFloat(dav.split("Version/")[1]) || ( ( parseFloat(dav.substr(idx+7)) >= 419.3 ) ? 3 : 2 ) || 2;

this should be as follows

d.isSafari = parseFloat(dav.split("Version/")[1]) || ( ( parseFloat(dav.substr(idx+7)) >= 419.3 ) ? 2 : 3 ) || 2;


All I had to change was 3 : 2 to 2 : 3.


Run the following in FireBug? to see the outcome...

// Declare the navigator.appVersion strings
var safari204 = "5.0 (Macintosh; U; PPC Mac OS X; en) AppleWebKit/418.8 (KHTML, like Gecko) Safari/419.3";
var safari312 = "5.0 (Macintosh; U; Intel Mac OS X 10_4_11; en) AppleWebKit/525.18 (KHTML, like Gecko) Version/3.1.2 Safari/525.22";
var safari312Windows = "5.0 (Windows; U; Windows NT 5.1; en-US) AppleWebKit/525.19 (KHTML, like Gecko) Version/3.1.2 Safari/525.21";

// Calculate the 'idx' value for each navigator.appVersion string
var idxWhenSafari204 = Math.max(safari204 .indexOf("WebKit"), safari204 .indexOf("Safari"), 0); // Comes out to 75
var idxWhenSafari312 = Math.max(safari312 .indexOf("WebKit"), safari312 .indexOf("Safari"), 0); // Comes out to 100
var idxWhenSafari312Windows = Math.max(safari312Windows .indexOf("WebKit"), safari312Windows .indexOf("Safari"), 0); // Comes out to 93

// Display the values BEFORE the conditional change
console.info(parseFloat(safari204.split("Version/")[1]) || ( ( parseFloat(safari204.substr(idxWhenSafari204+7)) >= 419.3 ) ? 3 : 2 ) || 2);
console.info(parseFloat(safari312.split("Version/")[1]) || ( ( parseFloat(safari312.substr(idxWhenSafari312+7)) >= 419.3 ) ? 3 : 2 ) || 2);
console.info(parseFloat(safari312Windows.split("Version/")[1]) || ( ( parseFloat(safari312Windows.substr(idxWhenSafari312Windows+7)) >= 419.3 ) ? 3 : 2 ) || 2);

// Display the values AFTER the conditional change
console.warn(parseFloat(safari204.split("Version/")[1]) || ( ( parseFloat(safari204.substr(idxWhenSafari204+7)) >= 419.3 ) ? 2 : 2 ) || 2);
console.warn(parseFloat(safari312.split("Version/")[1]) || ( ( parseFloat(safari312.substr(idxWhenSafari312+7)) >= 419.3 ) ? 2 : 2 ) || 2);
console.warn(parseFloat(safari312Windows.split("Version/")[1]) || ( ( parseFloat(safari312Windows.substr(idxWhenSafari312Windows+7)) >= 419.3 ) ? 2 : 2 ) || 2);

Results are as follows:

BEFORE
3
3.1
3.1

AFTER
2
3.1
3.1

comment:7 Changed 11 years ago by jgarfield

Sorry, had a typo in the code to try in FireBug?, should be as follows...

// Declare the navigator.appVersion strings
var safari204 = "5.0 (Macintosh; U; PPC Mac OS X; en) AppleWebKit/418.8 (KHTML, like Gecko) Safari/419.3";
var safari312 = "5.0 (Macintosh; U; Intel Mac OS X 10_4_11; en) AppleWebKit/525.18 (KHTML, like Gecko) Version/3.1.2 Safari/525.22";
var safari312Windows = "5.0 (Windows; U; Windows NT 5.1; en-US) AppleWebKit/525.19 (KHTML, like Gecko) Version/3.1.2 Safari/525.21";

// Calculate the 'idx' value for each navigator.appVersion string
var idxWhenSafari204 = Math.max(safari204 .indexOf("WebKit"), safari204 .indexOf("Safari"), 0); // Comes out to 75
var idxWhenSafari312 = Math.max(safari312 .indexOf("WebKit"), safari312 .indexOf("Safari"), 0); // Comes out to 100
var idxWhenSafari312Windows = Math.max(safari312Windows .indexOf("WebKit"), safari312Windows .indexOf("Safari"), 0); // Comes out to 93

// Display the values BEFORE the conditional change
console.info(parseFloat(safari204.split("Version/")[1]) || ( ( parseFloat(safari204.substr(idxWhenSafari204+7)) >= 419.3 ) ? 3 : 2 ) || 2);
console.info(parseFloat(safari312.split("Version/")[1]) || ( ( parseFloat(safari312.substr(idxWhenSafari312+7)) >= 419.3 ) ? 3 : 2 ) || 2);
console.info(parseFloat(safari312Windows.split("Version/")[1]) || ( ( parseFloat(safari312Windows.substr(idxWhenSafari312Windows+7)) >= 419.3 ) ? 3 : 2 ) || 2);

// Display the values AFTER the conditional change
console.warn(parseFloat(safari204.split("Version/")[1]) || ( ( parseFloat(safari204.substr(idxWhenSafari204+7)) >= 419.3 ) ? 2 : 2 ) || 2);
console.warn(parseFloat(safari312.split("Version/")[1]) || ( ( parseFloat(safari312.substr(idxWhenSafari312+7)) >= 419.3 ) ? 2 : 3 ) || 2);
console.warn(parseFloat(safari312Windows.split("Version/")[1]) || ( ( parseFloat(safari312Windows.substr(idxWhenSafari312Windows+7)) >= 419.3 ) ? 2 : 3 ) || 2);

comment:8 Changed 11 years ago by jgarfield

Forget the above posts...We finally narrowed it down...Leave to Peller to finish ;)

comment:9 Changed 11 years ago by Adam Peller

Resolution: fixed
Status: newclosed

(In [14317]) Fixes #6744. Really just a problem with the comparison. 419.3 should be Safari2, anything greater Safari3 (for now). Thanks, jgarfield.

Note: See TracTickets for help on using tickets.