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

Implement /core/browse-happy #173

Merged
merged 3 commits into from
Feb 22, 2025
Merged

Implement /core/browse-happy #173

merged 3 commits into from
Feb 22, 2025

Conversation

chuckadams
Copy link
Contributor

Pull Request

What changed?

  • Added /core/browse-happy/1.1 that does nothing but return a blank page.
  • Renamed CatchAllController to PassThroughController to make it more obvious what it does.

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.

@chuckadams chuckadams requested review from afragen, AmnestyAM, costdev and a team February 22, 2025 17:12
@chuckadams chuckadams merged commit c92baae into main Feb 22, 2025
1 check passed
@chuckadams chuckadams deleted the chuck/ac-45/browse-happy branch February 22, 2025 17:25
Comment on lines +5 to +6
// 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.
Copy link
Collaborator

@costdev costdev Feb 22, 2025

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.

Copy link
Contributor Author

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.

Copy link
Collaborator

@costdev costdev Feb 23, 2025

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.

Copy link
Contributor Author

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.

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.

Implement Endpoint: /core/browse-happy
3 participants