Bug 43982

Summary: A flaw in detecting mobile WebKit in WebKitSite/misc/WebKitDetect.js
Product: WebKit Reporter: Robin Qiu <robin.qiu>
Component: WebKit WebsiteAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: charles.wei, commit-queue, levin, mrowe, staikos, timothy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://webkit.org/misc/WebKitDetect.html
Attachments:
Description Flags
Patch v1
kenneth: review-
patch v2 none

Robin Qiu
Reported 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.
Attachments
Patch v1 (1.23 KB, patch)
2010-08-13 13:06 PDT, Robin Qiu 		kenneth: review-
DetailsFormatted DiffDiff
patch v2 (1.37 KB, patch)
2010-08-31 07:31 PDT, Robin Qiu 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Robin Qiu
Comment 1 2010-08-13 13:06:32 PDT
Created attachment 64368 [details] Patch v1
Adam Barth
Comment 2 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.
Robin Qiu
Comment 3 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?
David Levin
Comment 4 2010-08-17 18:28:39 PDT
Before landing, please add a bug link to your ChangeLog.
Kenneth Rohde Christiansen
Comment 5 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 "?
Robin Qiu
Comment 6 2010-08-31 07:31:41 PDT
Created attachment 66048 [details] patch v2 Change " Mobile/" to " Mobile\\b". Add bugzilla link.
Adam Barth
Comment 7 2010-08-31 19:28:30 PDT
Comment on attachment 66048 [details] patch v2 ok
Robin Qiu
Comment 8 2010-09-01 08:36:17 PDT
(In reply to comment #7) > (From update of attachment 66048 [details]) > ok Thanks for your review. :)
WebKit Commit Bot
Comment 9 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>
WebKit Commit Bot
Comment 10 2010-09-06 02:57:26 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.