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

[css-view-transitions-1] Prose to modify pseudo-trees #8113

Closed
jakearchibald opened this issue Nov 21, 2022 · 9 comments · Fixed by #8126
Closed

[css-view-transitions-1] Prose to modify pseudo-trees #8113

jakearchibald opened this issue Nov 21, 2022 · 9 comments · Fixed by #8126
Labels
css-view-transitions-1 View Transitions; Bugs only

Comments

@jakearchibald
Copy link
Contributor

When adding/removing regular elements from the DOM, we can use defined terms like pre-insert, and remove.

I don't think similar things exist for pseudo-element trees?

Right now we're using language like "append somePesudoElement to someOtherPseudoElement", but the term "append" isn't linked, so it's a bit of a hand-wave.

@fantasai @tabatkins are you aware of any other specs that have ran into this?

If it isn't a problem generally, we could define a "view transition pseudo-element" dfn that has a "pseudo-element children" list, and say that a view transition pseudo-element must render its pseudo-element children in order.

@khushalsagar
Copy link
Member

I like the idea of using pre-insert, and remove. That's what we're currently doing. This also clarifies the DOM order of the child pseudo-elements which dictates their default paint order.

Irrespective of the exact terminology for adding/removing the elements, it should explicitly indicate when they are added/removed. As opposed to other pseudo-elements, which are generated if there is a style targeting them, the lifetime of these pseudo-elements depends on specific events in the view transition lifecycle. It doesn't depend on the style cascade. That difference allows flexibility to potentially support features (like :has) for these pseudo-elements because we don't have any circularities. So it's important to clarify that in the spec.

@jakearchibald
Copy link
Contributor Author

I like the idea of using pre-insert, and remove.

We can't use those directly, as they're about elements, not pseudo-elements.

I'll cook up something for pseudo-elements.

@tabatkins
Copy link
Member

I think it's fine to use the DOM terms. Our tree isn't precisely the DOM, but it's extremely close and can carry over sufficiently without ambiguity. If you really feel the need you can add some weasel language about "terms from the DOM specification that refer to elements, such as "pre-insert", are used in this specification to also refer to pseudo-elements" to the introduction.

@khushalsagar khushalsagar added the css-view-transitions-1 View Transitions; Bugs only label Nov 23, 2022
@jakearchibald
Copy link
Contributor Author

Hmm, I'd rather not use the DOM terms, as they handle things like mutation observers, custom elements, connected state etc etc which pseudos just don't need.

I spec'd the concept of thing that can have pseudo-element children https://github.com/w3c/csswg-drafts/pull/8126/files#diff-0dba7871d6723c905f615f6b30609aeae92030cde7c2a895cd80fa540731288eR202. I think this works?

@jakearchibald
Copy link
Contributor Author

Here are the options I see:

Option 1: Use DOM's append etc definitions

I don't think this is a good idea, because those algorithms are not designed for pseudo-elements. Anyone following that definition will make incorrect assumptions or be confused.

Option 2: Leave it as a hand-wave & file an issue with the pseudo-element spec

Just use "append", "remove", and "descendant pseudos" as unlinked term, with an Issue: pointing to a GitHub issue for the pseudo-element spec to define them.

Option 3: Add a stub spec for pseudo-children to the VT spec, with an issue noting it should be moved to the pseudo-element spec

This is what #8126 currently does (except the issue noting it should be moved, but I'll add that).

This would also give us somewhere to specify :only-child.


I kinda like the pattern of option 3, "spec locally in a way that works locally. Then aim to move it into a more general spec". But I'm ok with option 2 too.

@khushalsagar
Copy link
Member

I was rooting for option 1 because there are other spec concepts which lean on the term "node" and the concept of a node tree that the pseudo-element tree needs to integrate with. Flat tree is one of those. Uses of it assume that this includes pseudo-elements. For instance, the contain spec here.

I'm not sure if the term node in flat tree intends to reference the node concept. It would be a problem if it does because pseudo-elements don't strictly support that definition (the Node idl is not exposed).

So if we go with option 1, we can clarify that tree abiding pseudo-elements are nodes except the IDL is not web-exposed. Then use the already defined terms for append/remove for nodes. And all other specs which talk about nodes work as-is.

Or we go with the "Pseudo-element parents" definition in #8126, which talks about pseudo-elements having a list of pseudo-element children. Then add a point to the Flat tree spec which includes pseudo-element trees in that traversal. Hopefully including it there clarifies how the box tree is generated and then everything else just works. This is what option 3 would build towards.

Now that I've read through the PR again, I quite like the way a pseudo-element tree is explained. It does feel like pulling in the DOM node concept will bring in functionality which was never intended to be supported by pseudo-elements. If the general idea is ok with css-pseudo editors, I'm good with option 3.

@fantasai @astearns what do you think? Other that VT pseudo-elements, this proposal would allow ::before to be explained as a pseudo-element parent since it can be the originating element for ::marker in the case of ::before::marker.

@annevk
Copy link
Member

annevk commented Nov 30, 2022

That seems like a bug in the definition of flat tree. And has had me confused quite a few times already. Please do actually create definitions and models that stand on solid ground and don't only work if you're familiar with the work that went into them.

E.g., using pre-insert on a non-node results in undefined behavior as far as specification go and is just not acceptable.

@fantasai
Copy link
Collaborator

fantasai commented Dec 6, 2022

Fwiw, I think the approach I would take is to define the view transition pseudo-elements as forming a tree, and define what that tree is by description rather than by algorithm. In general I prefer describing what the structure is to describing how to build it; I just find definitions easier to understand than trying to compile and run piles of algorithms in my head. :/

(Think about how you describe the view transition tree to the CSSWG: you don't walk us through all the algorithms for building it; you say that there's a tree of pseudo-elements, the root is ::view-transition, and it has these children, and they have those children... Of course the spec has to be more precise, but it can often do that that by being more detailed rather than taking a fundamentally different approach.)

Sometimes it's not possible to do that, but in that case I usually fully describe the target model first, and write up the algorithms to build it as an expansion of that definition. Having the ultimate model defined descriptively puts the algorithm into context: we know what we're trying to do, and why, and how it all fits together in the end.

Anyway, our goal is to optimize understandability of the spec without sacrificing precision. In that light, defining what a tree structure is, the fact that its node have parents, what a descendant is, etc. is unnecessary. These are all commonly understood concepts in our field, so there's no actual spec ambiguity to clear up by defining them. Therefore, creating an algorithmic definition of them just makes the spec longer and harder to read.

The question to ask is, where is the spec actually ambiguous? And clear that up. Afaict all you need to define is that you're building a tree of pseudo-elements, and that appending one pseudo-element to another one means appending it to its child list. You can make that a local definition; same with a pre-insert term, if you need one; “remove” should work fine here with just its English definition.

The reason DOM needs to define everything in detailed algorithms is because it it has complex input arguments to process and has to time various observable side-effects of the tree-building process such as events. We're not dealing with that here, so we can take shortcuts like relying on commonly understood definitions of common words.

@fantasai @tabatkins are you aware of any other specs that have ran into this?

I'm not aware of any other CSS spec that tries to build a pseudo-element tree. :) Let's do things locally here, and if we need to generalize it later we can refactor the concept.

@jakearchibald
Copy link
Contributor Author

In general I prefer describing what the structure is to describing how to build it;

Could we achieve the best of both by placing non-normative summaries before algorithms? I could also use https://dom.spec.whatwg.org/#trees. The reason I didn't, is it allowed me to be explicit that ::view-transition-old and new cannot have child pseudos, but I could just state that.

The reason DOM needs to define everything in detailed algorithms is because it it has complex input arguments to process and has to time various observable side-effects of the tree-building process such as events. We're not dealing with that here

One of the advantages of having linked terms for these things is Bikeshed makes it easy to see where particular things happen. Eg, if a document has a "view-transition root pseudo-element", a ::view-transition, which defines when the ::view-transition is rendered as part of the document, then I can navigate the spec by looking for references of that term, which shows me where it's set and unset. I'd like to retain that benefit, rather than going for something unlinked like "the ::view-transition is in the document from this point".

Does that sound ok?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-view-transitions-1 View Transitions; Bugs only
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants