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.

Sign up for GitHub

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

Jump to bottom

Render-blocking: the definition of "created by its node document's parser" is a little bit vague #10145

Open
cathiechen opened this issue Feb 19, 2024 · 5 comments
Open

Render-blocking: the definition of "created by its node document's parser" is a little bit vague #10145

cathiechen opened this issue Feb 19, 2024 · 5 comments
Labels
topic: parser

Comments

@cathiechen
Copy link

What is the issue with the HTML Standard?

In the Spec, to define implicitly potentially render-blocking there are:
For style element:

A style element is implicitly potentially render-blocking if the element was created by its node document's parser.

For link element:

The default type for resources given by the stylesheet keyword is text/css.
A link element of this type is implicitly potentially render-blocking if the element was created by its node document's parser.

For the script element:

A script element el is implicitly potentially render-blocking if el's type is "classic", el is parser-inserted, and el does not have an async or defer attribute.

We emphasize that the parser should be created by Document, this constraint looks a little bit vague to me.

And found in Gecko codebase, there are several parsing originations:

enum FromParser {
  NOT_FROM_PARSER = 0,
  FROM_PARSER_NETWORK = 1,
  FROM_PARSER_DOCUMENT_WRITE = 1 << 1,
  FROM_PARSER_FRAGMENT = 1 << 2,
  FROM_PARSER_XSLT = 1 << 3
};

Would it be more efficient to put the constraint on the parsing originations? For instance, <style> element parsed in the parser for fragment should not be implicitly potentially render-blocking?

WDYT? @emilio @xiaochengh @noamr @zcorpan
@emilio
Copy link
Contributor

emilio commented Feb 19, 2024

In particular, it's easy to track whether an element came from the parser, but not so much which document it came from (that's what the spec says roughly to do).

In particular, current behavior in Gecko for implicitly render-blocking stylesheets is not what the spec says: if you remove and then append a stylesheet back via script, it doesn't remain render-blocking.

It seems blink keeps track of whether the stylesheet came from the parser but not from which parser / document.

@xiaochengh
Copy link
Contributor

xiaochengh commented Feb 20, 2024 •
edited

For instance, <style> element parsed in the parser for fragment should not be implicitly potentially render-blocking?

Agreed, this shouldn't be render-blocking.

The current spec is basically a translation of Blink's implementation. Blink's element creation has a flag created_by_parser, and it makes an element render-blocking if the flag is set.

But the flag is also set for fragment parsing... I'll make a test case later to see if there's a bug.

Regarding the spec, the intention is to block on elements directly created by parsing the main resource without involving any scripting. Probably the same as FROM_PARSER_NETWORK in Gecko?

Edit: I'm not sure if an element created by document.write() should be render-blocking. We need some test cases to compare behaviors of different browsers.

@cathiechen
Copy link
Author

cathiechen commented Feb 20, 2024 •
edited

For instance, <style> element parsed in the parser for fragment should not be implicitly potentially render-blocking?

Agreed, this shouldn't be render-blocking.

The current spec is basically a translation of Blink's implementation. Blink's element creation has a flag created_by_parser, and it makes an element render-blocking if the flag is set.

But the flag is also set for fragment parsing... I'll make a test case later to see if there's a bug.

Regarding the spec, the intention is to block on elements directly created by parsing the main resource without involving any scripting. Probably the same as FROM_PARSER_NETWORK in Gecko?

Tested in the following case, it seems FROM_PARSER_NETWORK also include the sub resource, however, we could distinguish them by the nodeInfo, so it would work.

<!DOCTYPE html>
<body>
    <iframe src="./style-001.html" width="800" height="600"></iframe>
</body>
</html>

The iframe:

<!DOCTYPE html>
<head>
<style>
    @import url(resources/empty_style.css?stylesheet-inline-imported);
</style>
</head>
<body>
<div id="target">
    style blocking attr is set to render
</div>
</body>

Edit: I'm not sure if an element created by document.write() should be render-blocking. We need some test cases to compare behaviors of different browsers.

I don't have a strong opinion about this, but nodes created by document.write are involved in the script, no?

<!DOCTYPE html>
<head>
<meta charset="UTF-8">
<title>Should `document.write` style element be implicitly render-blocking?</title>
<script>
    document.write("<style>@import url('support/target-red.css?pipe=trickle(d1)');</style>");
</script>
</head>
<body>
    <div class="target">hello</div>
</body>
</html>

@emilio
Copy link
Contributor

emilio commented Feb 20, 2024

I think an element should be render-blocking if it's parser-inserted, but should stop being render-blocking as soon as it is detached from its tree... document.write-injected stuff should probably be render-blocking. I think that roughly matches Gecko's current (and historical) behavior, FWIW.

But also, implicitly render-blocking links and such are a bit more subtle. Gecko doesn't block rendering etc on stylesheets with media attributes that don't match for example, or for alternate stylesheets, and I don't think we'd want to start doing that.

@emilio emilio added the agenda+ To be discussed at a triage meeting label Feb 20, 2024
@xiaochengh
Copy link
Contributor

I think an element should be render-blocking if it's parser-inserted, but should stop being render-blocking as soon as it is detached from its tree...

Agreed.

I originally wanted to make "implicit render-blockingness" a stateless thing that can be determined from an element itself. It seems that it has to be maintained as an internal flag on the element that:

  • Is initially true if the element is created by parsing the main resource or content added by document.write
  • Is initially false in all other cases
  • Is set to false if the element is ever detached for any reason

Gecko doesn't block rendering etc on stylesheets with media attributes that don't match for example, or for alternate stylesheets

This part should be fine (or trivially fixable). For example, this part says we block rendering on a <style> only if its media attribute matches. Adding alternate should be trivial.

@past past mentioned this issue Feb 22, 2024
Closed
@past past mentioned this issue Mar 13, 2024
Closed
@past past removed the agenda+ To be discussed at a triage meeting label Mar 14, 2024
@past past mentioned this issue Mar 15, 2024
Closed
emilio added a commit to emilio/html that referenced this issue Mar 20, 2024
@emilio
[render-blocking] Fix implicitly-render-blocking definition.
46598a0 
As per discussion in whatwg#10145.
@emilio emilio mentioned this issue Mar 20, 2024
Open
5 tasks
emilio added a commit to emilio/html that referenced this issue Mar 20, 2024
@emilio
[render-blocking] Fix implicitly-render-blocking definition.
6171a58 
As per discussion in whatwg#10145.
@past past mentioned this issue Mar 28, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
topic: parser
Milestone
No milestone
Development

No branches or pull requests
5 participants
@past @zcorpan @emilio @cathiechen @xiaochengh