Ticket #6744 (closed defect: fixed)

Opened 2 months ago

Last modified 2 weeks ago

dojo.isSafari doesn't work properly for Safari 2.0.4

Reported by: jgarfield Owned by: peller
Priority: normal Milestone: 1.2
Component: Core Version: 1.1.0
Severity: normal Keywords: dojo, isSafari, dojo.isSafari, core, browser sniff
Cc:

Description (last modified by peller) (diff)

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

Changed 2 months ago by peller

  • owner changed from anonymous to alex

Changed 2 months ago by peller

  • description modified (diff)

Changed 2 months ago by alex

  • status changed from new to assigned

Changed 3 weeks ago by dylan

  • milestone set to 1.2

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

Changed 2 weeks ago by peller

  • owner changed from alex to peller
  • status changed from assigned to new

code looks right to me. asking jgarfield for clarification.

Changed 2 weeks 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

Changed 2 weeks 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);

Changed 2 weeks ago by jgarfield

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

Changed 2 weeks ago by peller

  • status changed from new to closed
  • resolution set to fixed

(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.