-
Notifications
You must be signed in to change notification settings - Fork 176
Update manifest spec to consider manifest updates for icon urls to be cache-control:immutable #1199
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
base: main
Are you sure you want to change the base?
Conversation
| `name_localized`. | ||
| </li> | ||
| </ol> | ||
| <p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spec is binding, and our implementation also considers icons if any of them change.
perhaps something like:
The user agent SHOULD consider a [=manifest image resource=] updated if the {{ImageResource/src}} member has changed. If the {{ImageResource/src}} has not changed, the user agent MAY download the image and check for visual differences in some cases, like when other [=manifest image resources=] have changed.
And then in the later paragraph, we need to call out that security sensitive memger change
User agents SHOULD NOT automatically apply changes to
[=security-sensitive members=] without [=express permission=] from
the user. User agents MAY consider an icon unchanged if the user agent determines the visual difference is insignificant.
IDK if we need to mention cache-control immutable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User agents MAY consider an icon unchanged if the user agent determines the visual difference is insignificant.
I thought this was an implementation detail of Chromium and not to be speced. This was also not proposed in the explainer, does it make sense to spec it? I applied your changes nonetheless, kept this as an open question.
Spec is binding, and our implementation also considers icons if any of them change.
Same question as above, I thought this was an implementation detail and not something we have proposed in the explainer, which only suggests icon url changes. Should we still spec it according to implementation?
IDK if we need to mention cache-control immutable
Imo, it might be nicer to keep the explainer, PRD and the spec say the same thing. It makes it easier to follow the chain of research and understand why it was kept that way. Wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User agents MAY consider an icon unchanged if the user agent determines the visual difference is insignificant.
I thought this was an implementation detail of Chromium and not to be speced. This was also not proposed in the explainer, does it make sense to spec it? I applied your changes nonetheless, kept this as an open question.
The spec tries to be clear to both devs and user agents about what behavior should be expected and allowed. The first sentence is a contract for the user agent - if a url changes, the developer expects the user agent will be checking new urls for icons. However, this is not the full story - can the developer expect that this is the ONLY time a user agent might check an icon? No. So the second sentence communicates to devs that the icons can also be fetched in other circumstances, and gives flexibility to user agents to do so (like we do sometimes).
cache-control immutalble also specifies this flexibility, with some specific exmaples - if you want prior art for this happening
Spec is binding, and our implementation also considers icons if any of them change.
Same question as above, I thought this was an implementation detail and not something we have proposed in the explainer, which only suggests icon url changes. Should we still spec it according to implementation?
Unfortunately it looks like your old commit is gone (did you amend?) so I cannot see your old language. From my memory, the language was written in a way that would make our implementation non-spec-compliant. It was incorrect.
IDK if we need to mention cache-control immutable
Imo, it might be nicer to keep the explainer, PRD and the spec say the same thing. It makes it easier to follow the chain of research and understand why it was kept that way. Wdyt?
That is a good way of 'understanding' the concept. We could add a 'non-normative-note' section for that. But to formally describe the behavior of this in terms of that spec, we have to hook this into concepts and algorithms in there, which seems difficult. You might be able to ask another spec person like Marcos about exactly how to do this, but I'm not confident enough in my knowledge to do this in a way that is correct and also is understandable / thorough.
I can see having a non-normative note here helpful about cache-control immutable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the information about cache-control immutable as an aside, PTAL again!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added section about insignificant changes in latest commit, PTAL!
59632f6 to
fb693e5
Compare
This change (choose at least one, delete ones that don't apply):
Implementation commitment (delete if not making normative changes):
Commit message:
This change updates the manifest specification to consider manifest updates only if the icon urls have changed, mimicking the cache-control:immutable behavior.
Person merging, please make sure that commits are squashed with one of the following as a commit message prefix:
Preview | Diff