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

Add device-memory and network quality client hints to the list #725

Closed
wants to merge 3 commits into from

Conversation

tarunban
Copy link

@tarunban tarunban commented May 15, 2018

annevk: please take a look. This is the first PR where the new client hints are added to the spec.


Preview | Diff

@tarunban
Copy link
Author

/cc @yoavweiss @igrigorik

Copy link
Member

@igrigorik igrigorik left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@annevk
Copy link
Member

annevk commented May 15, 2018

This doesn't seem to ever set them in the request?

Also, I think we should start by fixing the architecture for what's there now first. Otherwise it'd be rather hard to write tests and such.

@annevk
Copy link
Member

annevk commented May 15, 2018

And also, thanks for your pull request! And please add your name to the Acknowledgments section (unless you don't want to).

@tarunban
Copy link
Author

annevk: I am working on fixing the architecture in the spec, but that's taking a bit longer. I thought I would get this out first. Since my blink-dev i2s requires all this yak shaving anyways, I am not sure if the order is super critical? Please let me know. Thanks!

@annevk
Copy link
Member

annevk commented May 16, 2018

@tarunban https://whatwg.org/working-mode#changes might help. We have a pretty high bar for changes these days. So apart from these additions, we'll also need tests and the tests need to be based on the specification. And at that point you need the infrastructure in place.

yoavweiss added a commit to yoavweiss/fetch that referenced this pull request Aug 21, 2018
This change aligns the processing of Client-Hints with the shipped
Chromium implementation, by changing the following:
* Clone the environment settings object's client-hints set to the request's
  client-hints set.
* Apply CH processing to all requests, rather than only subresource
  requests.
* Add processing of new CH headers, added in whatwg#725

It also renames "client-hints list" to "client-hints set", and changes
it to be a set, to match related HTML spec changes.
yoavweiss added a commit to yoavweiss/fetch that referenced this pull request Aug 21, 2018
This change aligns the processing of Client-Hints with the shipped
Chromium implementation, by changing the following:
* Clone the environment settings object's client-hints set to the
  request's client-hints set.
* Apply CH processing to all requests, rather than only subresource
  requests.
* Add processing of new CH headers, added in whatwg#725
* Remove `DPR` and `Viewport-Width` from headers that are sent by
  default for navigation requests.

It also renames "client-hints list" to "client-hints set", and changes
it to be a set, to match related HTML spec changes.
yoavweiss added a commit to yoavweiss/fetch that referenced this pull request Aug 21, 2018
This change aligns the processing of Client-Hints with the shipped
Chromium implementation, by changing the following:
* Clone the environment settings object's client-hints set to the
  request's client-hints set.
* Apply CH processing to all requests, rather than only subresource
  requests.
* Add processing of new CH headers, added in whatwg#725
* Remove `DPR` and `Viewport-Width` from headers that are sent by
  default for navigation requests.

It also renames "client-hints list" to "client-hints set", and changes
it to be a set, to match related HTML spec changes.
@annevk annevk added topic: client hints needs tests Moving the issue forward requires someone to write tests labels Aug 23, 2018
yoavweiss added a commit to yoavweiss/fetch that referenced this pull request Jan 17, 2019
This change aligns the processing of Client-Hints with the shipped
Chromium implementation, by changing the following:
* Clone the environment settings object's client-hints set to the
  request's client-hints set.
* Apply CH processing to all requests, rather than only subresource
  requests.
* Add processing of new CH headers, added in whatwg#725
* Remove `DPR` and `Viewport-Width` from headers that are sent by
  default for navigation requests.

It also renames "client-hints list" to "client-hints set", and changes
it to be a set, to match related HTML spec changes.
yoavweiss added a commit to yoavweiss/fetch that referenced this pull request Feb 12, 2019
yoavweiss added a commit to yoavweiss/fetch that referenced this pull request Mar 26, 2019
This change aligns the processing of Client-Hints with the shipped
Chromium implementation, by changing the following:
* Clone the environment settings object's client-hints set to the
  request's client-hints set.
* Apply CH processing to all requests, rather than only subresource
  requests.
* Add processing of new CH headers, added in whatwg#725
* Remove `DPR` and `Viewport-Width` from headers that are sent by
  default for navigation requests.

It also renames "client-hints list" to "client-hints set", and changes
it to be a set, to match related HTML spec changes.
yoavweiss added a commit to yoavweiss/fetch that referenced this pull request Mar 26, 2019
@annevk
Copy link
Member

annevk commented Aug 6, 2020

Closing per 6a644c6.

@annevk annevk closed this Aug 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs tests Moving the issue forward requires someone to write tests topic: client hints
Development

Successfully merging this pull request may close these issues.

3 participants