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

Un-deprecate navigator.platform (was: Replace Deprecated_Header in navigator.platform doc and adjust navigator.userAgentData.platform note wording) #14452

Merged
merged 9 commits into from
Mar 31, 2022

Conversation

sideshowbarker
Copy link
Collaborator

@sideshowbarker sideshowbarker commented Mar 30, 2022

Fixes #14429. For the navigator.platform article, this change does the following:

  • Drop the Deprecated_Header macro.
  • Add admonishments that navigator.platform should be avoided in favor of feature detection.
  • Relegate mention of (equally-bad) navigator.userAgentData.platform to just being a See also.
  • Add an example of the single known good case where using navigator.platform isn’t completely bad.

Related BCD change: mdn/browser-compat-data#15599

Previous/now-superseded issue description…
  • 4db8611 Adjust wording of note about navigator.userAgentData.platform
  • f73b2d9 Replace Deprecated_Header in navigator.platform doc

Given navigator.userAgentData.platform is currently only supported in Blink, we shouldn’t be strongly “recommending” it; so 4db8611 drops the “recommended” and instead just points out navigator.userAgentData.platform as an alternative — while changing the “not yet supported by some major browsers” wording to instead more accurately state it’s “not yet supported by most browsers”.

And given that navigator.platform is not formally deprecated by the spec, f73b2d9 replaces the Deprecated_Header macro and its boilerplate text with some more-appropriate alternative pseudo-boilerplate text that’s intentionally broadly worded in such as way as to not be specific to just the navigator.platform case but instead worded for the general case where we want to warn web developers to avoid using a particular feature.

See also #12017 (comment).

- Don’t say that navigator.userAgentData.platform is “recommended”; it’s
  sufficient to just point out that it exists as an alternative.
- Since it’s so far only in Blink, say “not yet supported by **most**
  browsers” rather than “not yet supported by some major browsers.

See #14429
For the navigator.platform article, this change replaces the
Deprecated_Header macro and its boilerplate text with some
more-appropriate alternative pseudo-boilerplate text.

See #14429
@sideshowbarker sideshowbarker requested a review from a team as a code owner March 30, 2022 02:16
@sideshowbarker sideshowbarker requested review from wbamberg and removed request for a team March 30, 2022 02:16
@github-actions github-actions bot added the Content:WebAPI Web API docs label Mar 30, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Mar 30, 2022

Preview URLs

Flaws

URL: /en-US/docs/Web/API/Navigator/platform
Title: navigator.platform
on GitHub
Flaw count: 2

  • macros:
    • wrong xref macro used (consider changing which macro you use)
    • wrong xref macro used (consider changing which macro you use)

External URLs

URL: /en-US/docs/Web/API/Navigator/platform
Title: navigator.platform
on GitHub

No new external URLs

(this comment was updated 2022-03-31 12:21:10.854391)

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I complained about this already, I'm happy to see this! Approving but leaving open in case @hsivonen has a comment.

files/en-us/web/api/navigator/platform/index.md Outdated Show resolved Hide resolved
Co-authored-by: wbamberg <will@bootbonnet.ca>
@hsivonen
Copy link
Contributor

Thanks. I suggest dropping both instances of the word "yet".

@sideshowbarker
Copy link
Collaborator Author

Thanks. I suggest dropping both instances of the word "yet".

👍 Made it so in 57fdcf0

@sideshowbarker sideshowbarker marked this pull request as draft March 30, 2022 07:57
@sideshowbarker
Copy link
Collaborator Author

sideshowbarker commented Mar 30, 2022

Marking this as Draft so that we make sure not to merge it until we also get resolution on some of the details discussed in #14429 (comment)

@hsivonen
Copy link
Contributor

On further thought, instead of just suggesting the code be updated while also mentioning a Blink-specific API for also doing the discouraged thing it would make sense to mention that in general, Web developers should feature-detect relevant things when applicable and not care about the platform as a general rule.

@sideshowbarker sideshowbarker changed the title Replace Deprecated_Header in navigator.platform doc and adjust navigator.userAgentData.platform note wording Un-deprecate navigator.platform (was: Replace Deprecated_Header in navigator.platform doc and adjust navigator.userAgentData.platform note wording) Mar 31, 2022
sideshowbarker added a commit to sideshowbarker/browser-compat-data that referenced this pull request Mar 31, 2022
Given the spec change at whatwg/html#7762 we no
longer have justification for marking navigator.platform as deprecated.

Related MDN change: mdn/content#14452
@sideshowbarker sideshowbarker force-pushed the sideshowbarker/navigator.platform-discouraged branch from 3a31b6a to 0d3b545 Compare March 31, 2022 08:13
@sideshowbarker
Copy link
Collaborator Author

sideshowbarker commented Mar 31, 2022

OK, in light of the spec change at whatwg/html#7762, I’ve gone ahead and completely re-worked this patch to instead do the following:

  • Drop the Deprecated_Header macro.
  • Add admonishments that navigator.platform should be avoided in favor of feature detection.
  • Relegate mention of (equally-bad) navigator.userAgentData.platform to just being a See also.
  • Add an example of the single known good case where using navigator.platform isn’t completely bad.

Related BCD change: mdn/browser-compat-data#15599

On further thought, instead of just suggesting the code be updated while also mentioning a Blink-specific API for also doing the discouraged thing it would make sense to mention that in general, Web developers should feature-detect relevant things when applicable and not care about the platform as a general rule.

In the updated patch, I added some language which tries to give that advice.

So @hsivonen and @wbamberg, if you could both review the updated patch, I’d appreciate it.

@sideshowbarker sideshowbarker marked this pull request as ready for review March 31, 2022 08:16
Fixes #14429.

For the navigator.platform article, this change does the following:

- Drop the Deprecated_Header macro.
- Add admonishments that navigator.platform should be avoided in favor
  of feature detection.
- Relegate mention of (equally-bad) navigator.userAgentData.platform to
  just being a "See also".
- Add an example of the single known good case where using
  navigator.platform isn’t completely bad.

Related BCD change: mdn/browser-compat-data#15599

Related spec change: whatwg/html#7762
@sideshowbarker sideshowbarker force-pushed the sideshowbarker/navigator.platform-discouraged branch from 0d3b545 to 5d2d85b Compare March 31, 2022 08:18
@sideshowbarker sideshowbarker force-pushed the sideshowbarker/navigator.platform-discouraged branch from a51835f to e0e202c Compare March 31, 2022 08:30
@teoli2003
Copy link
Contributor

@sideshowbarker @hsivonen Feel free to ask for my review when this is ready to land. That way I know you are done and we can abide by our rule that this is not landed by the author of the PR.

@sideshowbarker
Copy link
Collaborator Author

@sideshowbarker @hsivonen Feel free to ask for my review when this is ready to land. That way I know you are done and we can abide by our rule that this is not landed by the author of the PR.

Thanks — will do

Copy link
Contributor

@hsivonen hsivonen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I can't actually pick the "Approve" option, because I'm not an official reviewer for this repo.

files/en-us/web/api/navigator/platform/index.md Outdated Show resolved Hide resolved
@sideshowbarker
Copy link
Collaborator Author

@teoli2003 Having gotten @hsivonen’s r+ on this, I think it’s ready to merge. I still hope @wbamberg can take a look at it post-merge and if there’s anything he notices that we missed, I’ll be happy to do a follow-up PR

Copy link
Contributor

@teoli2003 teoli2003 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a tiny change on the first line to make it coherent with the whole of MDN.

files/en-us/web/api/navigator/platform/index.md Outdated Show resolved Hide resolved
Co-authored-by: Jean-Yves Perrier <jypenator@gmail.com>
Copy link
Contributor

@teoli2003 teoli2003 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm merging. We can do more changes in follow-up PRs if needed.

Thanks, @hsivonen, @sideshowbarker and @wbamberg to have this sorted out! 🎉

@teoli2003 teoli2003 merged commit f440461 into main Mar 31, 2022
@teoli2003 teoli2003 deleted the sideshowbarker/navigator.platform-discouraged branch March 31, 2022 13:17

> **Note:** The recommended alternative to this property is {{domxref("NavigatorUAData.platform", "navigator.userAgentData.platform")}}. However, {{domxref("NavigatorUAData.platform", "navigator.userAgentData.platform")}} is not yet supported by some major browsers, and the specification which defines it has not yet been adopted by any standards group (specifically, it is not part of any specification published by the W3C or WHATWG).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sideshowbarker @teoli2003 Does it make sense to remove the info/links to NavigatorUAData.platform?

I understand the desire to point to feature detection, and the reasoning why navigator.platform shouldn't be deprecated.

But despite the spec change to navigator.platform my take is that you would be much better off using NavigatorUAData.platform. The user hints are more fully designed and specified - and while they aren't on all platforms, neither is the value of navigator.platform reliable on any platform.

Copy link
Collaborator Author

@sideshowbarker sideshowbarker Mar 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sideshowbarker @teoli2003 Does it make sense to remove the info/links to NavigatorUAData.platform?

I think it doesn’t make sense to remove it completely — which is why I moved it to being a See Also — but I think given the current state of things, it doesn’t make sense to give it as much prominence as it had.

I think giving it that amount of prominence made sense when we had navigator.platform marked as Deprecated — because in that case, we actually had people asking, OK, if this is deprecated, then what’s the alternative?

But as the fact that #14429 was raised kind of shows, we create a different problem when when we give this particular alternative the kind of prominence we were giving it — especially if we say it’s recommended.

But despite the spec change to navigator.platform my take is that you would be much better off using NavigatorUAData.platform. The user hints are more fully designed and specified

One thing we need to recognize is the people who work on non-Blink/Chrome browsers projects are not super happy if MDN promotes the use of some feature to developers when the current reality of that feature is that in practice it’s a Blink-only feature — even when that feature has a well-intentioned actively-maintained spec.

But it doesn’t matter how good the spec is if other browser projects aren’t implementing and it and aren’t even showing interest. And in the specific case of Client Hints, there was even at one time some discussion about having https://mozilla.github.io/standards-positions/ list is a Harmful.

It’s now marked “non-harmful” there but anyway we need to recognize that the Mozilla engineering team does not seem to be any interest in ever implementing Client Hints.

And while the Safari/WebKit team also doesn’t seem opposed to ever implementing Client Hints (see https://lists.webkit.org/pipermail/webkit-dev/2020-May/thread.html#31195), working on actually implementing doesn’t seem to be any significant priority for them — to the point that they haven’t even bothered to list it at https://webkit.org/status/.

my take is that you would be much better off using NavigatorUAData.platform

After looking through a lot of navigator.platform usages in Stack Overflow questions and answers in GitHub code, I can say that in nearly all cases where people are using navigator.platform in code, they are not using it for good reasons — and the kinds of things they are doing are things that we actually want to discourage developers from doing.

The same goes for ways in which somebody might use NavigatorUAData.platform — the underlying problem with both properties is that it’s fundamentally a bad idea to try to be finding out information about the user’s environment rather than only relying on feature detection.

I am aware of only one single case in which it makes sense to try and get information about the user’s platform — and that’s the deciding-on-which-modifier-key-to-advise-to-the-user case which I added an example for in the patch here.

Other than that one case, I’m not aware of even a single other best-practice case of something to do in code based on finding out information about the user’s platform — whether by using navigator.platform or with Client Hints.

and while they aren't on all platforms, neither is the value of navigator.platform reliable on any platform.

The key big differences are that navigator.platform is actually implemented in every browser engine (even if its reliability may vary), and navigator.platform is part of a standard that’s gone through the process of getting agreement and endorsements among multiple browser projects.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed explanation. I can get on board with it being a "see also".

@hsivonen
Copy link
Contributor

hsivonen commented Apr 1, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebAPI Web API docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue with "Navigator.platform": Cross-browser API marked deprecated and Chromium-only alternative suggested
5 participants