Bug 43982 - A flaw in detecting mobile WebKit in WebKitSite/misc/WebKitDetect.js
Summary: A flaw in detecting mobile WebKit in WebKitSite/misc/WebKitDetect.js
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Website (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL: http://webkit.org/misc/WebKitDetect.html
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-13 11:11 PDT by Robin Qiu
Modified: 2010-09-06 02:57 PDT (History)
6 users (show)

See Also:


Attachments
Patch v1 (1.23 KB, patch)
2010-08-13 13:06 PDT, Robin Qiu
kenneth: review-
Details | Formatted Diff | Diff
patch v2 (1.37 KB, patch)
2010-08-31 07:31 PDT, Robin Qiu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Robin Qiu 2010-08-13 11:11:11 PDT
http://trac.webkit.org/browser/trunk/WebKitSite/misc/WebKitDetect.js

It seems to be a popular script for detecting mobile WebKit but it has a flaw that makes it fail on Android and BlackBerry.  

The issue is that it looks for "Mobile/" but Android and BlackBerry use "Mobile" with no numbering after it.
Comment 1 Robin Qiu 2010-08-13 13:06:32 PDT
Created attachment 64368 [details]
Patch v1
Comment 2 Adam Barth 2010-08-14 23:06:58 PDT
Comment on attachment 64368 [details]
Patch v1

What character do they have after Mobile?  It seems like this might match MobileOpera or something silly like that.
Comment 3 Robin Qiu 2010-08-16 07:37:50 PDT
(In reply to comment #2)
> (From update of attachment 64368 [details])
> What character do they have after Mobile?  It seems like this might match MobileOpera or something silly like that.

It's a space, so, should we use " Mobile\b" as pattern?
Comment 4 David Levin 2010-08-17 18:28:39 PDT
Before landing, please add a bug link to your ChangeLog.
Comment 5 Kenneth Rohde Christiansen 2010-08-31 03:09:22 PDT
Comment on attachment 64368 [details]
Patch v1

r-, due to missing bug link in changelog, plus didn't you want to add the \b to make sure that you match "Mobile/" and "Mobile "?
Comment 6 Robin Qiu 2010-08-31 07:31:41 PDT
Created attachment 66048 [details]
patch v2

Change " Mobile/" to " Mobile\\b".
Add bugzilla link.
Comment 7 Adam Barth 2010-08-31 19:28:30 PDT
Comment on attachment 66048 [details]
patch v2

ok
Comment 8 Robin Qiu 2010-09-01 08:36:17 PDT
(In reply to comment #7)
> (From update of attachment 66048 [details])
> ok

Thanks for your review. :)
Comment 9 WebKit Commit Bot 2010-09-06 02:57:21 PDT
Comment on attachment 66048 [details]
patch v2

Clearing flags on attachment: 66048

Committed r66819: <http://trac.webkit.org/changeset/66819>
Comment 10 WebKit Commit Bot 2010-09-06 02:57:26 PDT
All reviewed patches have been landed.  Closing bug.