3rd-Eden / useragent

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

Microsoft Vista os not parsed correctly #120

Open lusarz opened 6 years ago

lusarz commented 6 years ago

I see that current output of:

useragent
  .parse('Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 6.0; WOW64; SLCC1; .NET CLR 2.0.50727; .NET4.0C; .NET4.0E)')
  .os

is: OperatingSystem { family: 'Windows', major: 'Vista', minor: '0', patch: '0' }

Some time ago it was: OperatingSystem { family: 'Windows Vista', major: '0', minor: '0', patch: '0' }

This cause errors in karma build: https://ci.appveyor.com/project/dignifiedquire/karma/build/1628/job/1a6b81i60l6wh52f

After some investigation I found that it seems to be related with this commit: https://github.com/3rd-Eden/useragent/commit/0e2892704e05194819d536b44d2116579fb944ff

mmermerkaya commented 6 years ago

This is because of an upstream change: https://github.com/ua-parser/uap-core/pull/286

@3rd-Eden Would you be willing to approve a PR that removes adding minor and patch values that are "0" (or null) to the stringified OS, so for example Vista would be stringified as "Windows Vista" instead of "Windows Vista.0.0"?

mgol commented 6 years ago

The changes in the regexps were intentional so it seems useragent needs to update its logic. I wonder how to best handle that. If a browser represents the OS as, for example, macOS 10.13.0 then the 0 at the end is meaningful and shouldn't be stripped.

Shouldn't useragent stop filling in non-provided fields with zeros? This would invalidate the following test: https://github.com/3rd-Eden/useragent/blob/c92567b6b5212436c015bd8b100968e836bf10fe/test/parser.test.js#L72-L85

This issue is a blocker for Karma as I understand.

mgol commented 5 years ago

@3rd-Eden could you weigh in if such a change would be accepted?

mgol commented 5 years ago

Ping @3rd-Eden