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

Update device client hint information #6097

Merged
merged 15 commits into from
Jun 25, 2021

Conversation

hamishwillee
Copy link
Collaborator

@hamishwillee hamishwillee commented Jun 18, 2021

This updates DPR HTTP header with directives section, and additional info from the last spec I can find in which it was mentioned (in the next version there is info that says headers were removed to their own specs, but I cannot yet find.

It also adds info about using Vary, and what happens if the value appears multiple times in response. That's from the spec.

I removed info about Accept-CH-Lifetime from this page, as that header is deprecated.

This is part of addressing #1408 and follows on from #5883.

@Elchi3 Feel free to review/merge this. If you don't, I'll continue to add the other device client hints docs until done. But hoping for sanity check. Note, I'm also after advice on how to find the spec for this.

@hamishwillee hamishwillee requested a review from a team as a code owner June 18, 2021 06:57
@hamishwillee hamishwillee requested review from mirunacurtean and removed request for a team June 18, 2021 06:57
@github-actions
Copy link
Contributor

github-actions bot commented Jun 18, 2021

Preview URLs

Flaws

Note! 5 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Web/HTTP/Headers
Title: HTTP headers
on GitHub
Flaw count: 4

  • macros:
    • /en-US/docs/Glossary/COEP does not exist
    • /en-US/docs/Glossary/COOP does not exist
    • /en-US/docs/Glossary/CORP does not exist
  • broken_links:
    • Can't resolve /en-US/docs/Web/API/Save_Data_API

External URLs

URL: /en-US/docs/Web/HTTP/Headers
Title: HTTP headers
on GitHub

No new external URLs


URL: /en-US/docs/Web/HTTP/Headers/Width
Title: Width
on GitHub


URL: /en-US/docs/Web/HTTP/Headers/Device-Memory
Title: Device-Memory
on GitHub


URL: /en-US/docs/Web/HTTP/Headers/Content-DPR
Title: Content-DPR
on GitHub


URL: /en-US/docs/Web/HTTP/Headers/Viewport-Width
Title: Viewport-Width
on GitHub


URL: /en-US/docs/Web/HTTP/Headers/DPR
Title: DPR
on GitHub

(this comment was updated 2021-06-25 01:42:58.975625)

Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

If there is no spec, then I think we should remove {{Specifications}}.

@hamishwillee hamishwillee requested a review from Elchi3 June 21, 2021 04:26
@hamishwillee
Copy link
Collaborator Author

hamishwillee commented Jun 21, 2021

@Elchi3 OK, this updated and I have added the other screen client hints. It is ready for re-review. There are bits that are "thin" but should be correct (I don't want to go into too much investigation on the deprecated API e.g. Content-DPR).
If you're OK with this it can be merged as the result of any answer below can be applied iteratively.

Following a helpful response on WHATWG matrix group I now know that the screen client hints moved to https://wicg.github.io/responsive-image-client-hints/. What it looks like happened is

But Chromium not yet made the change.

My assumption is that rather than attempt to rename/merge the new variants on the header we would just create new pages and new BCD entries for them and deprecate the old ones (@ddbeck). Is this correct?

What would be the trigger for this? Feels a bit early - not in any browser and there are still questions in the spec.

@ddbeck
Copy link
Contributor

ddbeck commented Jun 22, 2021

My assumption is that rather than attempt to rename/merge the new variants on the header we would just create new pages and new BCD entries for them and deprecate the old ones (@ddbeck). Is this correct?

This is right. The schema does have a notion of true renames (i.e., prefixes and alternative_name), but if it's not actually a rename—just a successor—then create a new feature.

What would be the trigger for this? Feels a bit early - not in any browser and there are still questions in the spec.

For BCD, avoid introducing all false features, unless there's evidence that implementation is in progress (i.e., if you can't link to an open implementation bug, then don't bother). We don't have a strict guideline about this, but mdn/browser-compat-data#10619 is open to answer that question.

Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

Thanks Hamish, great work! (again!)
Some nits inline.

If there is no implementation of the new Sec-CH-* headers, then I think documentation work should wait. The spec can change again and so it would be a catch-up game with something that isn't real yet and so just work for us with no user benefit.

files/en-us/web/http/headers/index.html Outdated Show resolved Hide resolved
files/en-us/web/http/headers/index.html Outdated Show resolved Hide resolved
files/en-us/web/http/headers/device-memory/index.html Outdated Show resolved Hide resolved
files/en-us/web/http/headers/device-memory/index.html Outdated Show resolved Hide resolved
files/en-us/web/http/headers/dpr/index.html Show resolved Hide resolved
files/en-us/web/http/headers/content-dpr/index.html Outdated Show resolved Hide resolved
files/en-us/web/http/headers/width/index.html Outdated Show resolved Hide resolved
hamishwillee and others added 6 commits June 25, 2021 11:17
Co-authored-by: Florian Scholz <fs@florianscholz.com>
Co-authored-by: Florian Scholz <fs@florianscholz.com>
Co-authored-by: Florian Scholz <fs@florianscholz.com>
Co-authored-by: Florian Scholz <fs@florianscholz.com>
Co-authored-by: Florian Scholz <fs@florianscholz.com>
@hamishwillee hamishwillee requested a review from Elchi3 June 25, 2021 01:41
@hamishwillee
Copy link
Collaborator Author

Thanks very much @Elchi3 . Accepted all your commits and integrated your suggestions. Ready for another look.

Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

Awesome! 👍

@Elchi3 Elchi3 merged commit aba4830 into mdn:main Jun 25, 2021
@hamishwillee hamishwillee deleted the http_device_clienthints branch June 26, 2021 08:53
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants