My favorites | Sign in
Project Logo
             
New issue | Search
for
| Advanced search | Search tips
Issue 8: Browser detection script is not correct for Safari/WebKit
2 people starred this issue and may be notified of changes. Back to list
Status:  New
Owner:  ----
Type-Defect
Priority-Medium


Sign in to add a comment
 
Reported by maciej, May 15, 2008
The user agent detection script here seems wrong in a number of ways for Safari/WebKit:

http://code.google.com/docreader/#p(doctype)s(doctype)t(ArticleUserAgent)


 isSafari = !isOpera && ua.indexOf('WebKit') != -1;

This seems to be checking for isWebKit, not isSafari. Web developers should probably check for 
WebKit rather than Safari almost all of the time, but this will identify non-Safari WebKit-based 
browsers as Safari. The property checking for Gecko is called isGecko, not isFirefox, after all.


    isKhtml = isKonqueror || isSafari;

Though WebKit is originally derived from KHTML, it is not accurate any more to consider it a form 
of KHTML and there are many differences between the engines. It would be better to check for 
something identifying the KHTML engine in the UA string (not sure if there is such a thing in 
Konqueror's UA string).


      } else if (isSafari) {
        // WebKit/125.4
        re = /WebKit\/(\S+)/;

This checks the WebKit version, not the Safari version. Granted, that is probably more useful to 
Web developers most of the time. The Safari source version (and for 3.0 and later also the 
"marketing" version) are present in the UA string separately.


    isMac = platform.indexOf('Mac') != -1;

This will incorrectly identify the iPhone as a Mac, since its UA string includes "(iPhone; U; CPU 
like Mac OS X; en)".


 /**
   * Whether the user agent is Konqueror. If this is true then KHTML is also
   * true. KHTML is the rendering engine that Konqueror and Safari uses.
   * @public
   * @type {Boolean}
   */
  /**
   * Whether the user agent is Safari. If this is true then KHTML is also
   * true. KHTML is the rendering engine that Konqueror and Safari uses.
   * @public
   * @type {Boolean}
   */

These two comments are factually incorrect, Safari uses WebKit, not KHTML.


In general the use of indexOf() here is very sloppy, since it will match anywhere in the string. 
Correct checks should use regexps for the context of the match.

Here is a link to correctly detect Safari and other WebKit-based browsers which does things 
correctly with regexps: <http://trac.webkit.org/wiki/DetectingWebKit>. A similar approach 
would probably work well with other browsers.
Sign in to add a comment

Hosted by Google Code