3rd-Eden / useragent

Useragent parser for Node.js, ported from browserscope.org
MIT License
897 stars 137 forks source link

PhantomJS is not detected correct #28

Closed dignifiedquire closed 11 years ago

dignifiedquire commented 11 years ago

Given the following user agent string:

Mozilla/5.0 (Macintosh; Intel Mac OS X) AppleWebKit/534.34 (KHTML, like Gecko) PhantomJS/1.6.0 Safari/534.34

This is the result

{ family: 'Safari',
  major: '0',
  minor: '0',
  patch: '0',
  source: 'Mozilla/5.0 (Macintosh; Intel Mac OS X) AppleWebKit/534.34 (KHTML, like Gecko) PhantomJS/1.6.0 Safari/534.34' }

But it should be

{ family: 'PhantomJS',
  major: '1',
  minor: '6',
  patch: '0',
  source: 'Mozilla/5.0 (Macintosh; Intel Mac OS X) AppleWebKit/534.34 (KHTML, like Gecko) PhantomJS/1.6.0 Safari/534.34' }
3rd-Eden commented 11 years ago

Thanks for reporting it! Just not sure if we should detect it as browser or as device. Because it is actually running a safari instance. But it runs it headless.

dignifiedquire commented 11 years ago

I would argue for browser as it is the program that is running and not the physical device. Also it is not Safari that PhantomJS uses but rather QTWebkit 1 so features do not map 1:1 with Safari. 

On Sun, Apr 7, 2013 at 4:05 PM, Arnout Kazemier notifications@github.com wrote:

Thanks for reporting it! Just not sure if we should detect it as browser or as device. Because it is actually running a safari instance. But it runs it headless.

Reply to this email directly or view it on GitHub: https://github.com/3rd-Eden/useragent/issues/28#issuecomment-16015634

dignifiedquire commented 11 years ago

I've looked a little bit more into it and the following works:

// lib/regexps.js
parser[0] = new RegExp("(PhantomJS)/(\\d+)\\.(\\d+).(\\d+)");
parser[1] = "PhantomJS";
parser[2] = 0;
parser[3] = 0;
parser[4] = 0;

But only if it's inserted before the Safari matcher 126.

3rd-Eden commented 11 years ago

Ill make sure its added in the next release — Twitter: @3rdEden Github: @3rd-Eden

On Sun, Apr 7, 2013 at 5:59 PM, Friedel Ziegelmayer notifications@github.com wrote:

I've looked a little bit more into it and the following works:

// lib/regexps.js
parser[0] = new RegExp("(PhantomJS)/(\\d+)\\.(\\d+).(\\d+)");
parser[1] = "PhantomJS";
parser[2] = 0;
parser[3] = 0;
parser[4] = 0;

But only if it's inserted before the Safari matcher 126.

Reply to this email directly or view it on GitHub: https://github.com/3rd-Eden/useragent/issues/28#issuecomment-16017523

dignifiedquire commented 11 years ago

I really would like to start using this in karma. If you tell me at which location you want the new regexp I can make a PR out of it.

3rd-Eden commented 11 years ago

@Dignifiedquire i'll add it today together with support for the IE string. I'll release it in 2~ hours

dignifiedquire commented 11 years ago

@3rd-Eden Thanks! That was quick :)

3rd-Eden commented 11 years ago

Released as 2.0.4

dignifiedquire commented 11 years ago

Thanks a lot