-
Notifications
You must be signed in to change notification settings - Fork 375
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
Slots in a flat tree #308
Comments
Where would the distributed nodes be projected? It would be problematic if they are projected inside the
|
Thanks for the feedback. I'd like to treat a slot, as well as an insertion point, as I guess someone doesn't like a slot element being left in the document composed tree, given a shadow root is not left in the document composed tree. |
I see. That's a pretty good argument for keeping the currently spec'ed behavior although it feels like we're working around the limitation of CSS. If we had a new display type like One thing I don't like about the current model is the discrepancy between the composed tree as exposed to CSS and the tree event dispatching takes place. Namely, the event path will include the slot element that is a deep ancestor of the target. |
I don't understand what you mean by this. A slot element is an element so it certainly can't be a document fragment. Moreover, as things stand, the concept of "slot" isn't well defined in the spec. It's a "defined location" according to the spec but there are no definitions of "defined"-ness or what a "location" is. It could be a (node, offset) pair like a boundary point or it could be a node. Whatever it is, it needs to be precisely defined. See issues #306 and #307. |
Thanks. I think we can close this issue. Please feel free to re-open this if required. One more note I can add here is that we might want to support the following use case in the future: <table>
SR
<slot name="thead"></slot>
<slot name="tr"></slot>
...
<thead slot="thead">...</thead>
<tr slot="tr">...</tr>
<tr slot="tr">...</tr>
... In this case, I guess no one is pleasant that |
I don't think we want to do this just for the styling use cases mentioned here. I think we just need to set slot { display: contents; } To put it another way, the composed tree is only useful to CSS box generation and the current distribution algorithm is trying to avoid box generation for slot element manually. And I'm saying that it's not necessary or desirable since box generation is inherently a CSS concern and should be dealt as such. We can preserve the behavior of not generating boxes (so that tables, flex boxes, ec... work) without having to do it manually in the algorithm by simply relying on a CSS feature such as One huge benefit of this approach is that the author can then override display property, e.g. to show borders around slot elements, if they wanted to, and removes one more magic from shadow DOM. |
FYI, we can get this benefit by wrapping a slot with an arbitrary element, like: Before:
After:
|
Right, but that's not really my point. The benefit of making this behavior reliant on CSS is that we'll decouple shadow DOM's distribution algorithm from the matter of box generation so that authors can figure it as they wish. |
I think we should try this. We've attempted this before, and failed for a reason I can't remember. I would love to explain |
I see. I think one good news is this is an implementation detail because users can't see the difference unless users try to style |
Isn't |
@annevk : I think @hayatoito : Mostly, yes. One big difference is that using UA stylesheet would mean that the author can override it with some other display type generate boxes on slot elements so it is observable to an extent. |
Now there are three benefits to take this approach:
|
I just remembered one other thing about unwrapping/flattening nodes. In Shadow DOM v0, we had this nice property that the tree-structural pseudo-classes operated on the composed tree (example here:https://gist.github.com/dglazkov/e8402f9ab18ad7ef2e7d). I guess the real question is how |
Since |
I'm thinking of adding the following changes to the spec:
A blocking issue might be how |
I forgot to mention that UA should have a UA style, I expect that Layout Tree doesn't change at all by default even if we add slots to the document composed tree. Can I assume that |
@esprehn pointed out to me that I may know understand all of the implications of using |
It should because box generation is separate from style inheritance
Contenteditable is a DOM concept. It won't be affected by display types. |
Hmm. That's not what I expected for Let me take another look later. |
Elements with All that
It "does nothing" visible, to the slot element itself, but the slot element still gets its |
The section 2.3 in the current shadow DOM says:
I think this statement needs to be removed/revised since a slot element is the parent node of its assigned nodes in the composed tree. |
Ideally slot elements should only be as magical as they need to be, and no more magical. I much prefer a world in which they all show up in the composed tree & the UA stylesheet sets display:contents on them to a world in which the engine magically hides them from style resolution. |
Marking this v1 since it seems important to agree on this. |
How can we agree on this issue? I would like to move this issue forward. There are still technical constraints to accept this proposal:
Given that, I would like to move this issue to v2. |
The concrete proposal is to use the default style of |
Yeah, I know {display: contents} is the hope, however, a lot of HTML elements's definition are not ready for {display: content}, I think. From https://drafts.csswg.org/css-display-3/#box-generation
e.g.
https://html.spec.whatwg.org/#the-tr-element Most of the definition of HTML elements use the parent directly in their definition and do not take care of the parent element's style. We have to update HTML Standard so that it will find the effective parent, skipping elements with {display: contents}. And we might have to update the parser too? Is WebKit ready for this? Unfortunately, Blink is not ready at all for {display: contents}. |
How else are we going to implement slot elements? One way or another, browser engines need to support semantics similar to In fact, it's fine for initial implementations of slot elements to not fully respect |
Thanks. I tend to agree, hoping web developers would not complain about Blink about this. |
My biggest concerns are:
Thus, I have to be careful to add a slot in a flat tree. If we support it, that means that Shadow DOM will fail if Is there any significant benefit of adding a slot in a flat tree, which is worth having such a great risk? I am not opposing to "slots in a flat tree", but I am afraid that it's too risky today, and today is not the day. @tabatkins, WDTY? Do you have a good feeling that "{display: contents}" will become successful?? |
In general, it's a good idea to minimize the number of new "magics" we introduce to the Web. Being able to describe/de-sugar slot element's behavior in terms of |
On more concrete examples, I already hit an interoperability problem with Chrome whereby which it doesn't respect |
I can't say whether it will succeed or not, as that depends on browser vendors implementing it. That said, I think Firefox has done so. |
@tabatkins, thanks. Can I confirm one more thing? Does the following HTML work as expected (from the view of the spec)?
This is the raised concern from @sorvell in #308 (comment). |
Yes, it works as expected, per spec. I think this works in Firefox right now if you want to test it. |
@tabatkins , thanks. I appreciate that. Then, it looks reasonable to support "add slots in a flat tree" with UA-style "slot { display:contents; }" in the spec. I'll update the spec. Since Blink has not supported "display: contents" yet and its engine requires a huge rewrite to support "a slots in a flat tree", Blink's initial implementation of v1 will not fully respect it. If someone has a concern about this decision, please let us know that. |
Looking at WebKit I think right now slotted elements inherit from the parent of the I'm very hesitant to ship with matching behavior, it doesn't seem like that's how display: contents was supposed to work. display: contents also has implications, for example are counter increments considered when declared on the slot? What about quoting level? Does slot::before and slot::after work? does display: contents disable ::before? (I hope so). What about slot::first-line ? I'd like to work out the details before we commit to making display: contents be a thing, and slot having a dependency on that technology. The current system is easy to explain, is just gone by the time styles are computed. There's no subtle rules for what does and does not apply. |
Correct, that is a broken implementation. Firefox has, afaik, a real implementation of display:contents.
All defined by https://drafts.csswg.org/css-display/#valdef-display-contents. Counters work on the element tree; they're not disturbed by box-tree manipulation. (If they were, then the 'order' property would mess with them too.) Quotes aren't very specific (they're defined by language lingering from CSS2, when we were much less explicit about the element/box/fragment divisions), but they should work the same. Pseudo-elements still generate as normal. |
Telecom agreement: We'd spec this as |
https://bugs.webkit.org/show_bug.cgi?id=149701 Reviewed by Antti Koivisto. Source/WebCore: Following discussions on Github discussions [1] [2], we're adding the default rule of `display: contents` on slot elements and making slot elements render its children when there are no assigned nodes [3]. Make these changes by attaching renderers on direct-children of slot elements when there are no assigned nodes during render tree construction. Note `display: contents` will be aded in webkit.org/b/149439. [1] WICG/webcomponents#317 [2] WICG/webcomponents#308 [3] WICG/webcomponents#308 (comment) Tests: fast/shadow-dom/css-scoping-shadow-slot-fallback.html fast/shadow-dom/shadow-layout-after-slot-fallback-changes.html * style/StyleResolveTree.cpp: (WebCore::Style::attachSlotAssignees): (WebCore::Style::detachSlotAssignees): (WebCore::Style::resolveSlotAssignees): LayoutTests: Added tests for fallback contents in slot elements. One of them could be safely submitted to CSS WG, and the other one is a style recalc test. * fast/shadow-dom/css-scoping-shadow-slot-fallback-expected.html: Added. * fast/shadow-dom/css-scoping-shadow-slot-fallback.html: Added. * fast/shadow-dom/shadow-layout-after-slot-fallback-changes-expected.html: Added. * fast/shadow-dom/shadow-layout-after-slot-fallback-changes.html: Added. Canonical link: https://commits.webkit.org/167842@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@190430 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Composition Algorithm defined in section 4.1 "unwraps" slots by re-distributing distributed nodes of a distributed slot by invoking Distributed Nodes Algorithm in section 3.3.
However, I don't think that should happen. In the world where re-distribution is explicit, there is no point in stripping out slot elements. It's better to treat a slot element like any other element and re-distribute it on its entirely. This will make the distribution/composition algorithm much simpler and allows slot element to be styled properly.
@hober @othermaciej @annevk @travisleithead
The text was updated successfully, but these errors were encountered: