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

Should <link rel="stylesheet"> work inside shadow DOM? #530

Closed
domenic opened this issue Jul 7, 2016 · 48 comments

Comments

Projects
None yet
9 participants
@domenic
Copy link
Contributor

commented Jul 7, 2016

Right now it uses "in a document", meaning we are not sure whether it should be "in a document tree" or "connected": https://html.spec.whatwg.org/multipage/semantics.html#link-type-stylesheet:concept-link-obtain

I think we should allow this (i.e. switch to "connected"). If we allow <style>, why not <link>?

@rniwa

This comment has been minimized.

Copy link
Contributor

commented Jul 7, 2016

Also see #526

@hayatoito

This comment has been minimized.

Copy link
Member

commented Jul 8, 2016

@rniwa

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2016

I don't think restricting link element to the document tree makes much sense. We should allow it in shadow trees.

@travisleithead @annevk

@rniwa rniwa added the v1 label Jul 8, 2016

@rniwa

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2016

On my second thought, I do recall a discussion about not allowing loading of external scripts and stylesheets to discourage people from loading the same styles/scripts multiple times in a single page.

@annevk

This comment has been minimized.

Copy link
Member

commented Jul 8, 2016

We already decided to allow scripts. I'm fine with allowing this as well. Presumably long-term this will mostly be done through an API.

@rniwa

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2016

On the other hand, it might be wise to not allow stylesheets or scripts to be loaded in shadow tree until that we figure out what we want to do with the new thing though. Otherwise, we may end up tying ourselves to some undesirable constraints.

@TakayoshiKochi

This comment has been minimized.

Copy link
Member

commented Jul 8, 2016

As @annevk already pointed out, in long-term we would use constructable stylesheets
http://tabatkins.github.io/specs/construct-stylesheets/.

Just for historical reference, some discussions related to this:

@domenic

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2016

It seems like the argument against is that UAs are able to optimize <style> with the same text content appearing many times in the same document, but are not able to optimize <link rel="stylesheet"> with the same href.

Is that completely true? Even if the response has appropriate long-lived caching headers? I guess on initial page load with an umprimed cache it would probably still make a bunch of requests, which is bad?

@hayatoito

This comment has been minimized.

Copy link
Member

commented Jul 8, 2016

Yeah, I think UA can optimize it in theory, but such an optimization is not guaranteed by the spec. As far as I remember, that was a concern from web developers.

Thus, we want to provide a standard way where such an optimization is strongly guaranteed.

@domenic

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2016

Hmm. I don't find that compelling, because as a web developer it seems like the same argument applies to multiple <style>s. That is, in theory it can be optimized so that the UA does not parse it multiple times or do other work, but in practice it is not guaranteed. (I think Apple has in fact stated they have such an optimization ready, whereas Google is pushing constructible stylesheets because they do not, if I recall previous discussions.)

Maybe it is an issue of degree of badness? Multi-parsing is not as bad as multi-fetching?

In general any asymmetry between <style> and <link rel="stylesheet"> is really surprising for web devs IMO...

@annevk

This comment has been minimized.

Copy link
Member

commented Jul 8, 2016

Yeah, and given <style>@import ...</style> it doesn't make sense to support one and not the other. We should just allow both.

@hayatoito

This comment has been minimized.

Copy link
Member

commented Jul 8, 2016

Thanks. I do not have a strong opinion on this issue, historically. :)
I am totally okay to allow <link rel="stylesheet"> if it is connected, hoping that web developers would use it in a good manner.

@trusktr

This comment has been minimized.

Copy link

commented Jul 12, 2016

It is definitely intuitive that <link> should work where <style> does. By the way, why not <style src=""> similar to script tags (seems even more intuitive)?

@hayatoito

This comment has been minimized.

Copy link
Member

commented Jul 15, 2016

Can we agree that <link rel="stylesheet"> should work if and only if it is connected?
I think it is reasonable, given that we have already allowed <style>@import ...</style>, which developers have been using as a loophole, I guess.

If we can agree on that, I will send "Intent to implement and ship: Allow <link rel="stylesheet"> which is connected" to blink-dev.

@trusktr

This comment has been minimized.

Copy link

commented Jul 15, 2016

Can we agree that should work if and only if it is connected?

Yes, and if it is disconnected then connected into another shadow tree then the styling should move there too.

@rniwa

This comment has been minimized.

Copy link
Contributor

commented Jul 15, 2016

I don't understand why we want to restrict to the connected shadow trees. What's the rationale?

Given style should apply to elements inside a disconnected shadow tree, and that forces @import to work, it seems rather inconsistent for link to not load in a disconnected shadow tree.

@rniwa

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2016

Also see the issue #526.

@trusktr

This comment has been minimized.

Copy link

commented Jul 18, 2016

@rniwa

Can we agree that should work if and only if it is connected?

I thought the conversation was about <link> being connected inside the shadow tree, not about the shadow tree being connected. Maybe it should wait for both to be true (makes sense): link element connected and shadow tree in a connected ancestor (because a shadow tree is always connected to it's host even if the host is in a disconnected tree, right?).

@domenic

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2016

No, a connected restriction here makes no sense... I don't understand why it's being discussed.

@domenic

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2016

That is, I think this should be governed by the normal rules for stylesheet links, with "in a document" replaced by "connected" to allow it to work inside shadow trees, and not worrying about its root's connected status, or the element being disconnected, or somehow "moving styles" whatever that means... or anything like that at all. This seems fairly simple... We should just spec it.

@trusktr

This comment has been minimized.

Copy link

commented Jul 18, 2016

or the element being disconnected and moved

What should happen if the <link> is moved to another shadow tree?

@domenic

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2016

This is all covered by the CSS scoping spec.

@trusktr

This comment has been minimized.

Copy link

commented Jul 18, 2016

Gotcha, so I assume exactly the same as with <style>.

But, <link> has a network aspect. If the <link>is connected to a document fragment, should it still fetch the resource? Or wait until inserted into real DOM?

@trusktr

This comment has been minimized.

Copy link

commented Jul 18, 2016

I meant, if the <link> is connected to a shadow root inside a document fragment.

@domenic

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2016

Please read the above link I gave, mentally replacing "in a document" with "connected".

(You may also want to re-read the definition of connected, as the way you're using it doesn't seem to match any existing definition.)

@trusktr

This comment has been minimized.

Copy link

commented Jul 18, 2016

From earlier,

Can we agree that should work if and only if it is connected?

It's not clear what parts of <link> working we're even talking about.

Maybe it should just do everything it already does now (nothing different), and waiting to apply styles until it is connected, because only then can the we possibly know to scope style to a shadow tree.

@domenic

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2016

I think it's pretty clear, between HTML and CSS Scoping. If you disagree, I'd suggest waiting until we get a spec finished and then you can investigate in more detail.

@rniwa

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2016

Well, for document, yes. What we're discussing is what we're gonna do with shadow DOM, so there is no intention whatsoever. We're defining what that intended behavior would be here.

@domenic

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2016

Well, we already made the decision for a few of those (notably scripts), to just change "in a document" to "connected". In no case have we decided to start loading resources when the resource's shadow-including root is not a document.

It seems important, for example, that if you wrap a script or link or image or... in a shadow tree, it doesn't suddenly start making resource fetches. E.g. a custom element wrapping an image (like x-gif) should behave just like an image and not perform fetches until the custom element becomes connected (and thus the img in its shadow DOM becomes connected).

@rniwa

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2016

@annevk

This comment has been minimized.

Copy link
Member

commented Jul 18, 2016

I agree with @domenic. I don't think "in a document" should become "in a document" or "whose root is a shadow root" generally. "Connected" makes sense.

@smaug----

This comment has been minimized.

Copy link

commented Jul 18, 2016

Hmm, making to work when not connected? that sounds very surprising.
Since has had the requirement for the element to be in document before loading, that is what I'd expect for shadow DOM too (that element is connected to the document). That would give some consistency.

Would be super weird if creating a random (not in document) DOM subtree and link in it, wouldn't load the stylesheet, but then using that same subtree, on some element in it do el.shadowRoot.innerHTML = "<link rel=stylesheet href=..>"; would actually load the stylesheet.

So, yes, we need connected-ness here.

@rniwa

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2016

Okay, let's go with "connected" then.

@Zambonifofex

This comment has been minimized.

Copy link

commented Jul 18, 2016

Sorry if I’m misunderstanding something, but I think that only loading when connected could cause unexpected behavior if someone tries to inspect the computed styles of the elements inside the shadow tree.

@annevk

This comment has been minimized.

Copy link
Member

commented Jul 18, 2016

@Zambonifofex could you elaborate?

@Zambonifofex

This comment has been minimized.

Copy link

commented Jul 18, 2016

Well, if I have the following stylesheet being linked:

button
{
    color: rgb(12, 34, 56);
}

…and but is a button inside the shadow tree, I’d expect the following to always log "rgb(12, 34, 56)", even if the shadow tree isn’t in a document:

console.log(window.getComputedStyle(but).color);

(by the way, sorry if I’m using wrong terminology)

@domenic

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2016

Why would you expect that? It's not true for node trees today. Why would you expect the result to be different when the disconnected node tree is rooted at a shadow root, instead of being rooted at e.g. a <div> or a DocumentFragment?

@Zambonifofex

This comment has been minimized.

Copy link

commented Jul 18, 2016

I guess I feel that way because shadow roots limit the scope of the stylesheet just like documents do. I’d expect the same if the link is inside a document that isn’t being displayed to the user (i.e. one created by document.implementation.createDocument/createHTMLDocument).

@domenic

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2016

Ah, no, stylesheets are not fetched in such documents. (See whatwg/html#1516 and http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=4299)

@Zambonifofex

This comment has been minimized.

Copy link

commented Jul 18, 2016

@domenic then I guess it makes more sense to require “connectedness” to fetch stylesheets. Still, that’s not what I’d expect at all.

@trusktr

This comment has been minimized.

Copy link

commented Jul 18, 2016

It seems important, for example, that if you wrap a script or link or image or... in a shadow root, it doesn't suddenly start making resource fetches.

That sounds perfect.

@hayatoito

This comment has been minimized.

Copy link
Member

commented Jul 19, 2016

+1 for the current conclusion.

Thank you, folks. I am totally fine: <link ref=stylesheet href="..."> fetches a stylesheet if and only if it is connected (except for documents without browsing contexts).
It looks what I would like to say was already covered by others. :)

@domenic

This comment has been minimized.

Copy link
Contributor Author

commented Jul 19, 2016

I will try to spec this as soon as someone (cough, @annevk) reviews whatwg/html#1516

domenic added a commit to whatwg/html that referenced this issue Jul 19, 2016

annevk added a commit to whatwg/html that referenced this issue Jul 20, 2016

dstockwell pushed a commit to dstockwell/chromium that referenced this issue Aug 4, 2016

Allow <link rel=stylesheet> in a connected shadow tree
See w3c/webcomponents#530. We have relaxed the condition.

"Web-Facing Change PSA" mail in blink-dev is:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/b7g62d2yqWo/2itpAFafBgAJ

Regarding a browsing context, this CL does not change any behavior. That should be
another concern and should be investigated later.

See also w3c/webcomponents#535. "title= attribute" is
ignored completely for <link rel=stylesheet>.

BUG=630141

Review-Url: https://codereview.chromium.org/2177163002
Cr-Commit-Position: refs/heads/master@{#409728}
@olanod

This comment has been minimized.

Copy link

commented Nov 3, 2017

Hello! I have a couple of issues described in this SO question regarding FOUC and measurement of elements while a stylesheet being loaded, is anyone here willing to share some thoughts about it?

alice added a commit to alice/html that referenced this issue Jan 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.