Made matching AppleWebKit more relaxed about how WebKit is capitalized. #77

Merged
merged 3 commits into from Jan 21, 2013

Conversation

Projects
None yet
2 participants
@bramstein
Member

bramstein commented Jan 9, 2013

This is to deal with the mysterious LG phone which seemingly uses 'AppleWebkit' (lower-case k) in its useragent string. The change doesn't use a case-insensitive regex on purpose. I prefer to play it safe and just allow the k to be lower-case as the seems to be the only documented case of it being different.

Made matching AppleWebKit more relaxed about how WebKit is capatilize…
…d. This is to deal with the mysterious LG phone which seemingly uses 'AppleWebkit' (lower-case k) in its useragent string.
src/core/useragentparser.js
@@ -223,7 +223,7 @@ webfont.UserAgentParser.prototype.parseOperaUserAgentString_ = function() {
* @private
*/
webfont.UserAgentParser.prototype.isWebKit_ = function() {
- return this.userAgent_.indexOf("AppleWebKit") != -1;
+ return this.userAgent_.indexOf("AppleWebKit") != -1 || this.userAgent_.indexOf("AppleWebkit") != -1;

This comment has been minimized.

Show comment Hide comment
@seanami

seanami Jan 10, 2013

Contributor

Should we use a regular expression for this and potentially save some characters? I'd be fine with leaving it like this too.

@seanami

seanami Jan 10, 2013

Contributor

Should we use a regular expression for this and potentially save some characters? I'd be fine with leaving it like this too.

src/core/useragentparser.js
@@ -223,7 +223,7 @@ webfont.UserAgentParser.prototype.parseOperaUserAgentString_ = function() {
* @private
*/
webfont.UserAgentParser.prototype.isWebKit_ = function() {
- return this.userAgent_.indexOf("AppleWebKit") != -1;
+ return /AppleWeb(?:K|k)it/.test(this.userAgent_);

This comment has been minimized.

Show comment Hide comment
@seanami

seanami Jan 11, 2013

Contributor

Super-minor nitpick: This doesn't need to be a non-capturing group in this regex, right?

@seanami

seanami Jan 11, 2013

Contributor

Super-minor nitpick: This doesn't need to be a non-capturing group in this regex, right?

This comment has been minimized.

Show comment Hide comment
@bramstein

bramstein Jan 14, 2013

Member

Correct, just a habit I got into (don't capture if you're not going to use it.) In this case it is completely unnecessary though.

@bramstein

bramstein Jan 14, 2013

Member

Correct, just a habit I got into (don't capture if you're not going to use it.) In this case it is completely unnecessary though.

This comment has been minimized.

Show comment Hide comment
@seanami

seanami Jan 14, 2013

Contributor

Sounds like a pretty reasonable habit, actually. Certainly makes it easier when you decide that you do need to add a capturing group.

@seanami

seanami Jan 14, 2013

Contributor

Sounds like a pretty reasonable habit, actually. Certainly makes it easier when you decide that you do need to add a capturing group.

@seanami

This comment has been minimized.

Show comment Hide comment
@seanami

seanami Jan 11, 2013

Contributor

This looks good to me!

Contributor

seanami commented Jan 11, 2013

This looks good to me!

bramstein added a commit that referenced this pull request Jan 21, 2013

Merge pull request #77 from typekit/bs-relax-webkit-detection
Made matching AppleWebKit more relaxed about how WebKit is capitalized.

@bramstein bramstein merged commit 834cd8b into master Jan 21, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment