-
Notifications
You must be signed in to change notification settings - Fork 3k
Allow <link> in body for external resource links #616
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
Conversation
source
Outdated
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.
This should expand the two lines about itemprop to include "or the rel attribute is present and the created link is...". It should be both flow and phrasing content in those cases.
The lists of flow and phrasing content in 3.2.4.2 need to be updated, as do the "Element content categories" section at the bottom of the spec. |
@domenic ack, updated. |
This mostly LGTM. Two questions for other editors:
|
Re, stylesheets-in-body: note that the use cases are not scoped to stylesheets only. As I noted in the original description, this is also useful for pre{load,fetch,connect, render}, etc. Also, allowing rel=stylesheet in body doesn't say anything about how it affects rendering, which is what the meat of the discussion in 27303 is all about.. one answer is that it doesn't and it blocks rendering of the entire page. |
Should we change conformance if we don't know the processing requirements yet? Am a little uneasy about that. What does "created link" mean? Also, what |
Yeah I think I agree with @annevk that we should wait with this until we've settled on the behavior. |
Do we anticipate that being done soon? I don't think we should hold up this authoring-requirement change on it. It remains the case that using link in body is a useful thing for authors today, no matter the state of the processing model in the spec. Besides, it seems like the behavior is well-specced, even if Edge disagrees and Blink is planning to become non-interoperable too.
I think this refers to the spec's existing clause:
Things like |
|
Well, that seems kind of like a bad thing to allow in the body.... maybe we need a more nuanced definition? |
If we wanted to allow all external resource links though I think it would be better to just use that phrase directly. Something more akin to "if link has its itemprop attribute set or it is an external resource link". |
The behavior is being actively discussed in blink-dev, and if browsers end up implementing blocking parsing behavior, I think it would be good to specify that soon. Is it really useful for authors today when it still blocks all rendering in Safari/Chrome/Opera? |
Sounds reasonable to me.
Once again, let's not get hung up on the processing of As a developer I want to emit With regards to stylesheets, note that none of the browsers ignore rel=stylesheet in body. The difference is in whether those block painting of all or some subset of content - that's what the discussion on blink-dev and in 27303 is all about. Personally, I see these discussions as separate and non-blocking.
Why should these be restricted to metadata section? In practice all browsers will process them regardless of where they are in the document. As in, I get it, conceptually it is metadata and the best practice is to put it at the top, but there are valid cases where you might not know this information when generating the head content. |
I tend to agree with this. I am surprised people are wanting to block on the behavior changes Blink proposes.
Of course. But that's not what authoring conformance criteria are about. They're about best practices and making your source code easier to read for humans and getting the best experience (e.g. letting the browser show the favicon or alternate links or similar before it finishes parsing the whole document). Unlike stylesheets, there is no downside to placing these early; they don't block rendering.
I can't imagine any such cases. I'd prefer we wait until we receive author complaints (as we have done with style) before changing these requirements. |
The case is simple and very common (e.g. Google search):
In short, we encourage streaming, and requiring that all metadata is known upfront breaks that since you have to buffer the entire response. |
@igrigorik I'm not talking about preload and prefetch, but about icon, alternate, and search, mostly. Those are metadata and should be in the head, in my opinion. Here's my whole breakdown of what I think should be in the head vs. allowed in the body, going off the link types defined in HTML:
Maybe someone can convince me that icon belongs in the second category, then the division can be that external resource links are always allowed in the body. But it seems pretty bad to delay the favicon until later. I guess I could be persuaded to simply add a document-conformance "should" for icon being in the head... Editors, I'd like to move forward on this; it's bad form to keep a nice PR hanging in this manner. I don't think we need to block changing document conformance criteria on defining the rendering model for stylesheets; those seem like separate concerns. Can you weigh in with your feelings on my above division? Is "external resource links are allowed in the body" the right way to go, or do you also feel icon should be in the head, or do you have an entirely different set of criteria? |
I think I would be fine with author/help/license/next/prev/search appearing in the body too, in the name of streaming. icon seems somewhat bad since you want that kind of information early on (a reason why I think manifests are a bad idea) and sidebar we should just remove. |
OK, let's not block this. I think we should consider consumers also. rel=author isn't so much for browsers, but other tools. Are there tools that just look at As usual when content models change, double-check the various sections if they need changes (content-venn diagram, content models section, indices). |
These are my thoughts exactly. I think the real question is icon. I am leaning toward the solution of:
This seems most consistent with the idea that authors are allowed to do anything for performance/streaming/etc. (which only external resource links impact), but also preserves our intuition that it's better to get icons early on when loading the page. |
I don't think we should allow |
OK. I am comfortable with that too, although it requires more spec work. Should we create a separate piece of information about each link ("is allowed in body") that each link definition specifies? Or should we do something like "all external resource links except icon"? |
I think I prefer the former. It's a reminder to consider whether it makes sense to allow new types in body instead of having them be allowed by default. |
Sounds good to me. @igrigorik, are you up for making these modifications? If you'd rather one of us do it, that seems fine to me too, since we're the ones making this so complicated :). Just let us know. I think the idea would be:
|
👍 I can take a run at it early next week. |
Update https://html.spec.whatwg.org/multipage/dom.html#phrasing-content-2 and https://html.spec.whatwg.org/multipage/dom.html#flow-content-2
->
(Maybe "allowed in the body" should cover the |
Similarly change https://html.spec.whatwg.org/multipage/indices.html#element-content-categories (flow content and phrasing content rows) |
@zcorpan ack, updated. Hopefully we're getting closer :) |
source
Outdated
|
||
</div> | ||
|
||
<p>A <code>link</code> element may be <dfn>allowed in the body</dfn>, depending on its |
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.
s/may/can/
I think this looks pretty good. The wrapping is a bit off in some places, but one of us can take care of that when merging. |
I noticed a new issue. <link rel="icon pingback" href="..."> |
source
Outdated
<p>The <code data-x="rel-prefetch">prefetch</code> keyword may be used with <code>link</code>, | ||
<code>a</code>, and <code>area</code> elements. This keyword creates an <span data-x="external | ||
resource link">external resource link</span>.</p> | ||
<code>a</code>, and <code>area</code> elements, and such elements are <span>allowed in the body |
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.
s/and such/and, for <code>link</code> elements, such/
(Also don't have a line break between a word and a tag; do line breaks anywhere where there is a space instead.)
@zcorpan ack, updated and squashed. I reworded https://github.com/whatwg/html/pull/616/files#r53937890 a bit, hope that works. |
FWIW this looks good to me (aside from the tiiiny nit that the first character of the commit message should be capitalized ;). I see @zcorpan has taken over in-depth review duties so I will leave the final say to him. Just wanted to say I'm happy with it. |
External links augment the current document and should not be restricted to metadata content. For example, the server may want to emit a prefetch, preload, or similar link within body content based on the generated content (well after the head section was flushed). Similarly, the position of a rel=stylesheet link in body can act as an optimization and signal to the user agent that DOM content above it may be painted - see [1]. However, note that this update does not define or change how or when CSS is applied - that's a separate discussion. All current browsers allow and process external resource links in body, this update reflects what's implemented and being used by developers. [1] https://www.w3.org/Bugs/Public/show_bug.cgi?id=27303#c37
@domenic capitalized :) |
Changes LGTM but #616 (comment) is not addressed yet. |
This, to me, implies that we don't need additional logic for "unless all the keywords are allowed in the body". |
But the "allowed in the body" concept applies to the element. My reading is that multiple link types get ORed for "allowed in the body", not ANDed. (I can try to fix this myself in the next few days.) |
I added a new commit in #777 |
Some external links augment the current document and should not be restricted to metadata content. For example, the server may want to emit a prefetch link within body content based on the generated content (well after the head section was flushed). Similarly, the position of a rel=stylesheet link in body can act as an optimization and signal to the user agent that DOM content above it may be painted - see [1]. However, note that this update does not define or change how or when CSS is applied - that's a separate discussion. All current browsers allow and process external resource links in body, this update reflects what's implemented and being used by developers. [1] https://www.w3.org/Bugs/Public/show_bug.cgi?id=27303#c37 PR: #616
Thanks!! Landed as 179983e |
External links augment the current document and should not be restricted
to metadata content. For example, the server may want to emit a
prefetch, preload, or similar link within body content based on the
generated content (well after the head section was flushed).
Similarly, the position of a rel=stylesheet link in body can act as an
optimization and signal to the user agent that DOM content above it may
be painted - see [1]. However, note that this update does not define or
change how or when CSS is applied - that's a separate discussion.
All current browsers allow and process external resource links in body,
this update reflects what's implemented and being used by developers.
[1] https://www.w3.org/Bugs/Public/show_bug.cgi?id=27303#c37