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

Limit meta tag parsing to one root, original tag #103

Closed
wants to merge 4 commits into from
Closed

Conversation

arichiv
Copy link
Collaborator

@arichiv arichiv commented Mar 23, 2022

This also adds support for speculative HTML parsing, whose fetches should respect the tag as well.

w3ctag/design-reviews#702 (comment)


Preview | Diff

This also adds support for speculative HTML parsing, whose fetches should respect the tag as well.

w3ctag/design-reviews#702 (comment)
@arichiv arichiv requested a review from yoavweiss March 23, 2022 16:54
@arichiv arichiv self-assigned this Mar 23, 2022
index.bs Outdated Show resolved Hide resolved
index.bs Outdated
----------
At [=speculative fetch=], after the sub-step for the meta element "viewport" step add a new sub-step with the following:
"A meta element whose name attribute is an ASCII case-insensitive match for "accept-ch" (only if this element is in the root Document and has not been modified by any script execution)."
Copy link

Choose a reason for hiding this comment

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

What does "modified by script execution" mean? How would you expect someone to implement that? If the intention is to go find all the places in an implementation where a document can be modified by script execution, and have them flip some sort of boolean that you later consult... you need to find all those places in the spec, and do so!

index.bs Outdated
@@ -197,14 +201,13 @@ after step 6, add the following step:
Standard metadata names {#standard-metadata-names}
------------
For the section <a href="https://html.spec.whatwg.org/multipage/#standard-metadata-names">standard metadata names</a>,
add a subsection named `accept-ch` with the [=Accept-CH state|outlined explanation=].
add a subsection named `accept-ch` with the [=Accept-CH state|outlined explanation=]. Note that this should be run exactly once,
and only on an element in the root Document which has not been modified by any script execution.
Copy link

@domenic domenic Mar 24, 2022

Choose a reason for hiding this comment

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

This does not seem interoperably implementable. When exactly should it be run? What if there are multiple elements with accept-ch? What if there are zero; how are we supposed to run this exactly once? How does this overlap with the speculative parsing section; if it's already run there, should it still be run here? (Plus the problem mentioned above about "modified by script execution" not being a defined term.)

@domenic
Copy link

domenic commented Mar 24, 2022

I might be off base here, but are you trying to do something like the following? https://github.com/WICG/nav-speculation/blob/main/meta-processing.md Specifically the processing model at the bottom.

@arichiv
Copy link
Collaborator Author

arichiv commented Mar 25, 2022

I think the aim is similar? We want the browser to accept the meta tag at both the speculative and actual page rendering steps and apply the client hint delegations within to subsequent loads. However, only a single meta tag sent in the original HTML (pre-scripting) should be respected, and others should be ignored.

@domenic
Copy link

domenic commented Mar 25, 2022

I really encourage you to write a spec which mirrors the code you have implemented, so that others can follow that spec and also implement such code. I don't see how to go from what you have in this PR to anything like what you described in the comment you just posted.

@arichiv
Copy link
Collaborator Author

arichiv commented Apr 20, 2022

I'll be taking this up again as part of #108

@arichiv arichiv deleted the limits branch April 20, 2022 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants