Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(dep): Bump useragent to fix HeadlessChrome version #3201

Merged
merged 1 commit into from
Nov 5, 2018

Conversation

rkuzsma
Copy link
Contributor

@rkuzsma rkuzsma commented Nov 3, 2018

Bump useragent to 2.3.0 to pull in ua-parser/uap-core#263 in which Headless Chrome version detected to be 0.0.0.
Update tests to reflect latest useragent OS major version string identifiers.

Per ua-parser/uap-core#286 , useragent purposely sets the Windows Vista family to Windows and major version to Vista. The change to useragent was intentional, and the maintainer advises upstream consumers to update tests to reflect the change.

Per 3rd-Eden/useragent#120 , there is an open question to change the stringified OS to not include 0 for missing version numbers. For example:

family: 'Windows'
major: 'Vista'
minor:
patch:
patch_minor:

would display as "Windows Vista" instead of "Windows Vista.0.0".

That open question not withstanding, this commit is an incremental improvement: You can now see both Chrome Headless version and Windows version information in your console output. In general, useragent consumers that perform int() conversions on 'major' should be aware.

Fixes #2762

@mgol
Copy link
Contributor

mgol commented Nov 3, 2018

AFAIK Karma used to use a newer useragent version but reverted to an older one to avoid the Vista.0.0 issue but not showing a Chrome (even if only headless) version at all seems worse than this purely visual issue.

@mgol
Copy link
Contributor

mgol commented Nov 3, 2018

@rkuzsma Please make sure Travis passes, it's red right now (due to incorrect commit message format from what I see).

@mgol
Copy link
Contributor

mgol commented Nov 3, 2018

Please don't close this PR, though, as you did with #3199 & #3200 as that only splits the discussion and makes changes hard to follow. Instead, amend the commit and push via:

git push --force-with-lease

@rkuzsma
Copy link
Contributor Author

rkuzsma commented Nov 5, 2018

Thanks, I fixed the commit message and updated the PR. All checks are passing now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants