-
Notifications
You must be signed in to change notification settings - Fork 12
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
Implement /core/browse-happy #173
Conversation
// As far as I can tell, /core/browse-happy is a telemetry endpoint returning nothing that browsers hit directly. | ||
// Implementing it here checks off an item on the compatibility list, but it'll likely never see use in the real world. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Records data when a UA starts with WordPress/
, and if so, ~1 in 25 occasions.
Other than that, it returns the likes of this data when hit with this UA (which was a sample from MDN):
With no other parameters, serialized data is returned:
a:10:{s:4:"name";s:6:"Chrome";s:7:"version";s:13:"51.0.2704.103";s:8:"platform";s:5:"Linux";s:10:"update_url";s:29:"https://www.google.com/chrome";s:7:"img_src";s:43:"http://s.w.org/images/browsers/chrome.png?1";s:11:"img_src_ssl";s:44:"https://s.w.org/images/browsers/chrome.png?1";s:15:"current_version";s:2:"18";s:7:"upgrade";b:0;s:8:"insecure";b:0;s:6:"mobile";b:0;}
It also supports a jsonp
or callback
query arg, though I know AC isn't interested in supporting JSONP.
The core functionality of Browse Happy is to parse the UA, identifying the latest available version of the browser - Source, whether their version can be upgraded, and whether their version is considered secure. That's the bit that may be useful to copy if looking for the simplest clone of it, or a custom implementation using browserlist if looking for something not using hard-coded, ancient versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm comfortable leaving it as an empty stub then, unless there's something that actually consumes the response and cares if it isn't consumable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing errors if it just returns empty or even if the HTTP request fails.
wp_check_browser_version()
makes the call to the API, and wp_dashboard_browser_nag()
uses it to determine whether to show the notice - not unlike the ServeHappy API. - Just info about how it's used in WP Core. Personally I'm not totally sure the API has the same benefit it once had.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose in my next gardening pass I'll have to update the comment about it being hit directly by the browser and mention that the implementation is a stub. But yeah, both endpoints are about as useless as they come, so that's not going to be a priority.
Pull Request
What changed?
Why did it change?
Possibly the endpoint does more on a POST, but I think all it does is collect telemetry. I'm also pretty sure this endpoint is only hit by browsers, meaning AU won't rewrite it anyway. It's not impossible to intercept browser requests, but the juice isn't worth the squeeze, especially not for this endpoint.
Did you fix any specific issues?
Closes: #45
CERTIFICATION
By opening this pull request, I do agree to abide by the Code of Conduct and be bound by the terms of the Contribution Guidelines in effect on the date and time of my contribution as proven by the revision information in GitHub.