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

Block parser (but not rendering) on external CSS #9630

Open
jasonwilliams opened this issue Feb 14, 2016 · 6 comments
Open

Block parser (but not rendering) on external CSS #9630

jasonwilliams opened this issue Feb 14, 2016 · 6 comments
Labels

Comments

@jasonwilliams
Copy link
Contributor

@jasonwilliams jasonwilliams commented Feb 14, 2016

More of a question than an issue.
How does servo handle this currently?

Is this something servo plans to do?

Could be worth thinking about if the chrome team are planning to put it into the spec

More info below:

https://groups.google.com/a/chromium.org/forum/m/#!topic/blink-dev/ZAPP8aTnyn0
https://jakearchibald.com/2016/link-in-body/

@jdm
Copy link
Member

@jdm jdm commented Feb 14, 2016

From document.rs:

165     /// Number of stylesheets that block executing the next parser-inserted script
166     script_blocking_stylesheets_count: Cell<u32>,

We implement the model currently described in the spec (and implemented by Firefox too, I believe).

@jdm jdm added the A-content/dom label Feb 14, 2016
@jasonwilliams
Copy link
Contributor Author

@jasonwilliams jasonwilliams commented Feb 14, 2016

I managed to dig up some more information about how its been implemented here:

Stylesheets for HTMLLinkElements are now loaded by the resource task, triggered by the element in question. Stylesheets are owned by the elements they're associated with, which can be HTMLStyleElement, HTMLLinkElement, and HTMLMetaElement (for `).

Additionally, the quirks mode stylesheet (just as the user and user agent stylesheets a couple of commits ago), is implemented as a lazy static, loaded once per process and shared between all documents.

This all has various nice consequences:

  • Stylesheet loading becomes a non-blocking operation.
  • Stylesheets are removed when the element they're associated with is removed from the document.
  • It'll be possible to implement the CSSOM APIs that require direct access to the stylesheets (i.e., ~ all of them).
  • Various subtle correctness issues are fixed.

One piece of interesting follow-up work would be to move parsing of external stylesheets to the resource task, too. Right now, it happens in the link element once loading is complete, so blocks the script task. Moving it to the resource task would probably be fairly straight-forward as it doesn't require access to any external state.

543703e

@jasonwilliams
Copy link
Contributor Author

@jasonwilliams jasonwilliams commented Feb 14, 2016

@tschneidereit I would be interested to hear your thoughts on this, as i think you implemented this originally

@tschneidereit
Copy link
Contributor

@tschneidereit tschneidereit commented Feb 14, 2016

It'll be possible to implement the CSSOM APIs that require direct access to the stylesheets (i.e., ~ all of them).

That was exactly my motivation for working on this. I've since moved to implementing Promise in SpiderMonkey (and then adding support for that in Servo) because that blocks more things than CSSOM, but I have a, by now horribly bitrotted, wip of parts of CSSOM locally.

One piece of interesting follow-up work would be to move parsing of external stylesheets to the resource task, too. Right now, it happens in the link element once loading is complete, so blocks the script task. Moving it to the resource task would probably be fairly straight-forward as it doesn't require access to any external state.

Yes, that would be a nice follow-up. I had considered doing that in the first version but then decided to limit the scope where I could.

@Ms2ger refactored much of this to make it nicer, but I don't think his changes affect this particular task much.

@jasonwilliams
Copy link
Contributor Author

@jasonwilliams jasonwilliams commented Feb 14, 2016

@tschneidereit have you taken a read of https://jakearchibald.com/2016/link-in-body/ ?
How possible is that in Servo with what you mentioned?

@tschneidereit
Copy link
Contributor

@tschneidereit tschneidereit commented Feb 14, 2016

@tschneidereit have you taken a read of https://jakearchibald.com/2016/link-in-body/ ?

Apologies, I had totally misread the issue, and not seen the link.

How possible is that in Servo with what you mentioned?

It seems like it should be fairly straight-forward, as we're already blocking parsing for <script> tags, like Firefox does. Extending that to also block parsing, but not rendering of already-parsed content for <link rel='stylesheet'> nodes shouldn't be all that hard.

I also think that it's a pretty good idea.

@jasonwilliams jasonwilliams changed the title Block parser on external CSS Block parser (but not rendering) on external CSS Feb 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.