Fixes #196 - Add a Sec-CH-UA-Full-Version-List client hint #250

miketaylr · 2021-08-11T03:34:06Z

TODO:

deprecate Sec-CH-UA-Full-Version
should getHighEntropyHints return a FrozenArray for valueList?
update README
file corresponding CH Infra issue (to capture permissions policy), and coordinate landing that PR once this is ready
https://wicg.github.io/ua-client-hints/#create-an-arbitrary-version needs to be updated to take an argument as well

💥 Error: 400 Bad Request 💥

PR Preview failed to build. (Last tried on Sep 13, 2021, 9:15 PM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 CSS Spec Preprocessor - CSS Spec Preprocessor is the web service used to build Bikeshed specs.

🔗 Related URL

Error running preprocessor, returned code: 2.
FATAL ERROR: Couldn't load MDN Spec Links data for this spec.
Expecting value: line 1 column 1 (char 0)
 ✘  Did not generate, due to fatal errors

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

miketaylr · 2021-08-13T22:01:44Z

@erik-anderson mind providing some feedback on this PR?

yoavweiss · 2021-08-16T06:43:28Z

High-level question on this: Initially, @mikewest defined Sec-CH-UA so that when an opt-in is provided, its processing fills in the full version instead of the significant version. We later removed that in favor of Full-Version, to make processing simpler (to not special case Sec-CH-UA). Maybe that was a mistake?

mikewest · 2021-08-16T12:35:25Z

High-level question on this: Initially, @mikewest defined Sec-CH-UA so that when an opt-in is provided, its processing fills in the full version instead of the significant version. We later removed that in favor of Full-Version, to make processing simpler (to not special case Sec-CH-UA). Maybe that was a mistake?

I still like the original version. :)

But given the way things have evolved, I'm not sure it's worth rolling everything back. Chrome has shipped Sec-CH-UA for a while now, and changing its semantics might be difficult.

miketaylr · 2021-08-16T20:03:20Z

Initially, @mikewest defined Sec-CH-UA so that when an opt-in is provided, its processing fills in the full version instead of the significant version. We later removed that in favor of Full-Version, to make processing simpler (to not special case Sec-CH-UA).

Hmm, that's pretty interesting, I think I missed out on that entirely! (Or knew at one point and just forgot... 🙈 ).

Recapping for myself: the early design was if you requested ua (now it would have to be sec-ch-ua) you would get a full-versioned brandlist, rather than the default significant versioned one.

I think folks might request sec-ch-ua to get at (full!) versions without understanding that they get significant versions for free, so a different-enough token would probably be desirable were we to take that approach. Maybe sec-ch-ua-full-version(s) could have powered that behavior... but that ship has sailed. I don't think we want to make stumbling into requesting extra entropy to be so easy. It wouldn't' surprise me to find code where folks request the full version list and parse it to get at a significant version.

And looking back to #45, I can see @yoavweiss had the same concerns. WDYT, @yoavweiss? Do we go back to the magic behavior with a different token (we could re-use this proposalssec-ch-ua-version-list), or do we introduce a new hint?

One one hand, there's fewer headers over the wire if you want full-version via sec-ch-ua, on the other, it's kind of magical or confusing.

Also interested in feedback from @erik-anderson.

yoavweiss · 2021-08-17T12:23:32Z

One one hand, there's fewer headers over the wire if you want full-version via sec-ch-ua, on the other, it's kind of magical or confusing.

Yeah, I agree with that description of the tension. Given that requesting for the full version should be rather rare, let's go with less magic and confusion.

yoavweiss · 2021-08-17T12:24:48Z

README.md

@@ -12,7 +12,8 @@ a corresponding JavaScript API:
 * `Sec-CH-UA-Platform`
 * `Sec-CH-UA-Platform-Version`
 * `Sec-CH-UA`
-* `Sec-CH-UA-Full-Version`
+* `Sec-CH-UA-Full-Version` (deprecated in favor of `Sec-CH-UA-Version-List`)


Do we have use counters for Full-Version? Could we just change its semantics in place (instead of rename)?

Let me check on that. It would be nicer if we could...

OK, we might have missed the boat here with 1.6% of page loads, sadly 😿:

https://chromestatus.com/metrics/feature/timeline/popularity/3189

@yoavweiss looks like it's too high to change here. Shall we continue review assuming a new hint?

1.6% is an astoundingly high number. Do you have any insight into who's using this mechanism? Is there some large embedded resource that's causing this to show up on lots of sites, or are there large top-level sites opting in?

We know that YouTube has been experimenting with UA-CH (and sadly we caused a crash there, oops), and we have some other partners who have committed to doing some testing.

README.md

yoavweiss · 2021-08-19T07:03:47Z

index.bs


+1. Let |list| be a [=/list=], initially empty.
+1. Let |brands| be the result of running the [=create brands=] steps, with
+    [=user agent/significant version=] passed as an argument.


It's unclear if what is passed is the version itself, or an instruction to operate on that version type. I think you meant the latter, but it might be worthwhile to pass an internal enum, and then refer internally to the right version type based on that enum.

Also, right now both significant version and full version are defined as a single version. It may be worthwhile to adapt their descriptions to indicate that, unlike Highlander, there can be more than one.

it might be worthwhile to pass an internal enum, and then refer internally to the right version type based on that enum

This sounds probably like what we want to do, but I'm not sure how to spec it. Are you aware of any other similar usage in a spec I can learn from, or have some suggestions?

@yoavweiss does ac95343 look better from your perspective?

index.bs

yoavweiss

LGTM assuming we can't change semantics

README.md

erik-anderson

This addresses the concern I had in issue #196. Thanks!

miketaylr · 2021-08-30T19:28:08Z

This addresses the concern I had in issue #196. Thanks!

Thanks for the review! @erik-anderson any thoughts on Sec-CH-UA-Version-List vs Sec-CH-UA-Full-Version-List? Former is less chars to type, latter is more accurate/correct.

…s confusing

erik-anderson · 2021-08-30T20:41:17Z

This addresses the concern I had in issue #196. Thanks!

Thanks for the review! @erik-anderson any thoughts on Sec-CH-UA-Version-List vs Sec-CH-UA-Full-Version-List? Former is less chars to type, latter is more accurate/correct.

I don't think omitting it is less accurate, but it is indeed less verbose and may take someone staring at it slightly harder to understand that it provides full version numbers. I think it would be fine to omit the word "Full" and just spec it to return full versions; that's my slight preference.

…rsion

It should be more clear this is a type, and not a single value.

…ion-List

index.bs

yoavweiss

Looks good! A few comments that can make it even gooder

index.bs

yoavweiss · 2021-09-13T05:39:46Z

index.bs

+To <dfn abstract-op>return the `Sec-CH-UA-Full-Version-List` value for a request</dfn>, perform the
+following steps:
+
+1. Let |version type| be the type [=user agent/full version=].


You're using [=user agent/full version=] here as both the type passed and its value. It's a bit weird.
Maybe instead you can define an internal enum (or string or whatever) that you pass to the algorithm, and then inside the algorithm, split the behavior based on the enum's value?

Thanks - I think I finally get what you mean. Let's land this PR here and I filed #254 to fix this up (should be able to do that this week).

soffieswan041 · 2021-09-20T15:20:40Z

TODO:

deprecate Sec-CH-UA-Full-Version

should getHighEntropyHints return a FrozenArray for valueList?

update README

file corresponding CH Infra issue (to capture permissions policy), and coordinate landing that PR once this is ready

https://wicg.github.io/ua-client-hints/#create-an-arbitrary-version needs to be updated to take an argument as well

Error: 400 Bad Request

PR Preview failed to build. (Last tried on Sep 13, 2021, 9:15 PM UTC).

More

miketaylr added 7 commits August 9, 2021 16:20

Issue WICG#196 - Fix incorrect client hint in example

a3893b2

Issue WICG#196 - Refactor create brands to take a versionType arg

12b87f0

Issue WICG#196 - Add a Sec-CH-UA-Version-List client hint

3f9fa99

Issue WICG#196 - The version list should be a FrozenArray

281a8e8

Issue WICG#196 - Deprecate Sec-CH-UA-Full-Version

3366860

Issue WICG#196 - Update README

dd93ab3

Issue WICG#196 - Pass in |versionType| to "create an arbitrary version"

a742195

miketaylr marked this pull request as ready for review August 13, 2021 22:01

miketaylr requested review from amtunlimited and yoavweiss August 13, 2021 22:03

yoavweiss reviewed Aug 17, 2021

View reviewed changes

miketaylr requested a review from yoavweiss August 18, 2021 18:27

yoavweiss reviewed Aug 19, 2021

View reviewed changes

yoavweiss approved these changes Aug 27, 2021

View reviewed changes

erik-anderson reviewed Aug 27, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

erik-anderson reviewed Aug 27, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

erik-anderson approved these changes Aug 27, 2021

View reviewed changes

Issue WICG#196 - Use correct brand string for Microsoft Edge in example

88b7621

Issue WICG#196 - Make README comment on NavigatorUAData interface les…

1daf9ca

…s confusing

Issue WICG#196 - Refactor out a "Create a brand-version list" algorithm

01cc7d3

miketaylr force-pushed the issues/196/1 branch from d40b280 to 01cc7d3 Compare August 31, 2021 22:31

miketaylr added 2 commits September 3, 2021 10:36

Issue WICG#196 - Clarify that there can be more than 1 sig. & full ve…

6169842

…rsion

Issue WICG#196 - Rejigger algorithms that take a |version type|

ac95343

It should be more clear this is a type, and not a single value.

Issue WICG#196 - On second thought, let's call it Sec-CH-UA-Full-Vers…

48232f2

…ion-List

miketaylr mentioned this pull request Sep 9, 2021

Add Sec-CH-UA-Full-Version-List WICG/client-hints-infrastructure#85

Closed

miketaylr changed the title ~~Fixes #196 - Add a Sec-CH-UA-Version-List client hint~~ Fixes #196 - Add a Sec-CH-UA-Full-Version-List client hint Sep 9, 2021

miketaylr mentioned this pull request Sep 9, 2021

Fixes #85 - Add Sec-CH-UA-Full-Version-List bits WICG/client-hints-infrastructure#86

Merged

yoavweiss reviewed Sep 10, 2021

View reviewed changes

index.bs Outdated Show resolved Hide resolved

Issue WICG#196 - Fix creating a version list frozen array.

8ba2083

yoavweiss reviewed Sep 13, 2021

View reviewed changes

Issue WICG#196 - Update last instance of "version list" (and add full)

8cc707f

miketaylr mentioned this pull request Sep 13, 2021

Improve algorithms that take a version type #254

Closed

miketaylr merged commit cfb3c39 into WICG:main Sep 13, 2021

miketaylr deleted the issues/196/1 branch September 13, 2021 21:18

dontcallmedom mentioned this pull request Sep 16, 2021

FrozenArray in dictionaries #258

Closed

soffieswan041 mentioned this pull request Sep 20, 2021

Revert "Fixes #196 - Add a Sec-CH-UA-Full-Version-List client hint" #259

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #196 - Add a Sec-CH-UA-Full-Version-List client hint #250

Fixes #196 - Add a Sec-CH-UA-Full-Version-List client hint #250

miketaylr commented Aug 11, 2021 •

edited by pr-preview bot

Loading

miketaylr commented Aug 13, 2021

yoavweiss commented Aug 16, 2021

mikewest commented Aug 16, 2021

miketaylr commented Aug 16, 2021

yoavweiss commented Aug 17, 2021

yoavweiss Aug 17, 2021

miketaylr Aug 17, 2021

miketaylr Aug 18, 2021

miketaylr Aug 18, 2021

mikewest Aug 19, 2021

miketaylr Aug 19, 2021

yoavweiss Aug 19, 2021

miketaylr Sep 2, 2021

miketaylr Sep 9, 2021

yoavweiss left a comment

erik-anderson left a comment

miketaylr commented Aug 30, 2021

erik-anderson commented Aug 30, 2021

yoavweiss left a comment

yoavweiss Sep 13, 2021

miketaylr Sep 13, 2021

soffieswan041 commented Sep 20, 2021

Error: 400 Bad Request

Fixes #196 - Add a Sec-CH-UA-Full-Version-List client hint #250

Fixes #196 - Add a Sec-CH-UA-Full-Version-List client hint #250

Conversation

miketaylr commented Aug 11, 2021 • edited by pr-preview bot Loading

💥 Error: 400 Bad Request 💥

miketaylr commented Aug 13, 2021

yoavweiss commented Aug 16, 2021

mikewest commented Aug 16, 2021

miketaylr commented Aug 16, 2021

yoavweiss commented Aug 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yoavweiss left a comment

Choose a reason for hiding this comment

erik-anderson left a comment

Choose a reason for hiding this comment

miketaylr commented Aug 30, 2021

erik-anderson commented Aug 30, 2021

yoavweiss left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soffieswan041 commented Sep 20, 2021

Error: 400 Bad Request

miketaylr commented Aug 11, 2021 •

edited by pr-preview bot

Loading