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

Identify Chromium and Headless Chrome as "modern" browsers. #10334

Merged
merged 5 commits into from Nov 15, 2018

Conversation

Projects
None yet
3 participants
@abernix
Copy link
Member

commented Nov 14, 2018

This PR introduces changes to treat "Headless Chrome" and Chromium the same as Chrome in terms of identifying them as "modern" browsers (and thus serving them the modern, rather than legacy, bundles).

Best I can tell, the major version portion of Chromium versions has always tracked all the way through to Chrome Canary, Dev and Stable releases. The Chromium versioning documentation seems to back this up.

Furthermore, Headless Chrome isn't really a "different" Chrome — in fact, all recent versions of Chrome can be run with the --headless flag (yes, even the one on your — the reader's — local computer!) to be run without the visual element (and driven programmatically). However, when run with the --headless flag, Chrome switches its primary navigator.userAgent from Chrome/a.b.c.d to HeadlessChrome/a.b.c.d.

This was initially problematic since the useragent npm we use for parsing user agents didn't understand this designation, however, with the update of webapp's useragent npm in 058351b, headlesschrome will now have its version available from WebAppInternals.identifyBrowser, so we can accurately identify it and serve it the modern bundle.

Chromium on the other hand, hasn't been a prominent issue since on macOS the navigator.userAgent is very much the same as Chrome itself. For example, my own macOS Chromium identifies itself as:

> navigator.userAgent
< "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3611.0 Safari/537.36"

(Note, no presence of Chromium)

However, on my Linux Chromium, a different behavior introduces a second Chromium/ identifier before the Chrome/ version identifier with the same MAJOR.MINOR.BUILD.PATCH:

> navigator.userAgent
< "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/70.0.3538.77 Chrome/70.0.3538.77 Safari/537.36"

By updating useragent (058351b) and aliasing chromium to chrome (845c45a), both of these situations should be improved.

@abernix abernix force-pushed the abernix/update-useragent-for-modern-browsers branch from 8f887ce to 845c45a Nov 14, 2018

"version": "2.2.4",
"resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-2.2.4.tgz",
"integrity": "sha1-bGWGGb7PFAMdDQtZSxYELOTcBj0="
"version": "4.1.3",

This comment has been minimized.

Copy link
@abernix

abernix Nov 14, 2018

Author Member

This is a pretty major bump, but these shrinkwrap updates seem to have been necessary because of updates to the blaze submodule which webapp still depends on via its need for boilerplate-generator. I don't believe the lru-cache API has ever changed in any major way and tests all seem to be passing.

"version": "2.2.1",
"resolved": "https://registry.npmjs.org/useragent/-/useragent-2.2.1.tgz",
"integrity": "sha1-z1k+9PLRdYdei7ZY6pLhik/QbY4="
"version": "2.3.0",

This comment has been minimized.

Copy link
@abernix

abernix Nov 14, 2018

Author Member

This was the most important update in this shrinkwrap.

@hwillson
Copy link
Member

left a comment

Awesome @abernix - looks great!

@abernix abernix requested a review from benjamn Nov 14, 2018

@abernix abernix self-assigned this Nov 14, 2018

@benjamn benjamn force-pushed the abernix/update-useragent-for-modern-browsers branch from 845c45a to 849b63f Nov 15, 2018

benjamn added a commit that referenced this pull request Nov 15, 2018

benjamn added a commit that referenced this pull request Nov 15, 2018

@benjamn benjamn changed the base branch from devel to release-1.8.0.1 Nov 15, 2018

@benjamn benjamn added this to the Package Patches milestone Nov 15, 2018

@abernix
Copy link
Member Author

left a comment

I agree that's a responsible and defensive addition. Thanks, for the follow-up, @benjamn.

LGTM.

abernix and others added some commits Nov 14, 2018

Update `webapp`'s `useragent` npm to v2.3.0 to support HeadlessChrome.
Previously, while the `useragent` package was able to parse the User-Agent
for so-called "Headless Chrome" and generate a family of "HeadlessChrome",
it was unable to parse out the individual portions of the version number
(e.g. major, minor, patch).

For example, the following User-Agent (herein referred to as `userAgentAbove`):

```
Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/69.0.3497.100 Safari/537.36
```

Previously resulted in:

```
> require('useragent').lookup(userAgentAbove);

{
  family: 'HeadlessChrome',
  major: '0',
  minor: '0',
  patch: '0',
  /* ... */
}
```

With the newer version of `useragent`, these are now properly extracted and
set which will enable Meteor to treat Headless Chrome the same as Chrome in
a follow-up commit.  Now:

```
> require('useragent').lookup(userAgentAbove);

{
  family: 'HeadlessChrome',
  major: '69',
  minor: '0',
  patch: '3497'
  /* ... */
}
```
For modern bundles, treat Chromium and "Headless Chrome" the same as …
…Chrome.

Best I can tell, the major version portion of Chromium versions has always
tracked all the way through to Chrome Canary, Dev and Stable releases.
Since we observe the major version of Chrome in terms of identifying it as a
"modern browser", it seems to make sense to treat "Headless Chrome" and
Chromium in the same regard.

Interestingly, when the same Chrome as we all use on our machines is run
with the `--headless` flag, it switches its `navigator.userAgent` to
`HeadlessChrome/`, rather than `Chrome/`.

This was initially problematic since the `useragent` npm we use for parsing
user agents didn't understand this designation, however, with the update of
`webapp`'s `useragent` npm in 058351b, `headlesschrome` will now have its
version available from `WebAppInternals.identifyBrowser`, so we can
accurately identify it and serve it the modern bundle.

@benjamn benjamn force-pushed the abernix/update-useragent-for-modern-browsers branch from 849b63f to 65e44f6 Nov 15, 2018

@benjamn benjamn changed the base branch from release-1.8.0.1 to devel Nov 15, 2018

@benjamn benjamn merged commit 258d7f2 into devel Nov 15, 2018

5 of 18 checks passed

ci/circleci: Isolated Tests CircleCI is running your tests
Details
ci/circleci: Test Group 0 CircleCI is running your tests
Details
ci/circleci: Test Group 1 CircleCI is running your tests
Details
ci/circleci: Test Group 10 CircleCI is running your tests
Details
ci/circleci: Test Group 2 CircleCI is running your tests
Details
ci/circleci: Test Group 3 CircleCI is running your tests
Details
ci/circleci: Test Group 4 CircleCI is running your tests
Details
ci/circleci: Test Group 5 CircleCI is running your tests
Details
ci/circleci: Test Group 6 CircleCI is running your tests
Details
ci/circleci: Test Group 7 CircleCI is running your tests
Details
ci/circleci: Test Group 8 CircleCI is running your tests
Details
ci/circleci: Test Group 9 CircleCI is running your tests
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
CLA Author has signed the Meteor CLA.
Details
ci/circleci: Docs Your tests passed on CircleCI!
Details
ci/circleci: Get Ready Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@benjamn benjamn deleted the abernix/update-useragent-for-modern-browsers branch Nov 15, 2018

@benjamn benjamn restored the abernix/update-useragent-for-modern-browsers branch Nov 15, 2018

benjamn added a commit that referenced this pull request Nov 15, 2018

benjamn added a commit that referenced this pull request Nov 15, 2018

benjamn added a commit that referenced this pull request Nov 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.