Notes from spec review

[jakearchibald]

https://tabatkins.github.io/specs/css-shared-element-transitions/

---

::page-transition-outgoing-image( ) - One of these pseudo-elements exists for each element being animated by the page transition

Is that true in cases where there isn't an outgoing element? As in, the element only exists in the 'after' state?

---

If the page-transition pseudo-element exists, a new stacking context is created for the root and top layer elements. The page-transitioon layer is a sibling of this stacking context.

"transitioon" -> "transition"

Also, does this define any kind of order vs other things in the top layer?

---

Promise<any> prepare(AsyncFunction cb);

Should this be Promise<undefined>?

---

a struct with items named "outgoing image"

Would it be better to make "outgoing image" a definition for the parent object, so it can be referenced in a more strict way?

---

Additionally the following structures exist:

Should it be clearer that these are private slots on SameDocumentTransition?

---

If any SameDocumentTransition object in the document (this or any other)

I think we need to be clearer on how the document is chosen, as it matters when one iframe calls another iframe's implementation of SameDocumentTransition.

The path is usually via the relevant settings object of this.

---

If any SameDocumentTransition object in the document (this or any other) has a [[Phase]] internal slot set to a non-"idle" value, throw an InvalidStateException.

I don't think this is the behaviour we want. I think we either want to abort any currently running transition in favour of this one, or immediately abort this transition when it's prepared.

The current behaviour in the spec skips calling the callback, which prevents the DOM change from happening, which seems wrong given that transitions are an enhancement.

Same goes for the other early throws.

---

If multiple elements on the page have the same page transition tag, throw an InvalidStateException.

Need a stronger definition of "on the page". IMO it should refer to rendered elements or pseudo elements connected to the document. Although, I don't know if that definition includes things like ::backdrop on modals.

---

If multiple elements on the page have the same page transition tag, throw an InvalidStateException.

This line is duplicated.

---

For each element el with a non-none page transition tag

I think we need to be stricter about what this means. As in, limit it to document-connected elements that are rendered.

---

el does not have layout containment applied.
el does not forbid fragmentation.

Feels like "layout containment" and "fragmentation" should link to the definitions.

---

el does not have layout containment applied.
el does not forbid fragmentation.

Does the root automatically meet these conditions?

---

el is not rendered.

It feels like these elements should be filtered out rather than cause errors. For instance, it feels like I should be able to create a transition from:

<div style="page-transition-tag: foo">hello</div>
<div style="page-transition-tag: foo; display: none">world</div>

To

<div style="page-transition-tag: foo; display: none">hello</div>
<div style="page-transition-tag: foo">world</div>

---

Execute the following steps after step 14 of Update the rendering

Shouldn't it be after step 16 when the rendering is actually updated? Maybe put the content of the step in a note, in case steps are added/removed from the HTML spec.

---

Let tag be el’s page-transition-tag value.

Is there a stricter way to refer to the computed value here? I'm thinking of cases where the value is var(--foo).

---

Set capture’s "outgoing styles" to the following:

Maybe need to use a more formal type here. Eg, is it a style object, is it a struct? etc etc

---

Create transition pseudo-elements managed by this.

Nit: This is like a callback, so I don't think "this" can be referenced. Probably just needs to be set to a variable during the synchronous steps.

---

Suppress rendering opportunities for this’s Document.

Feels like we need a more formal definition for "Suppress rendering opportunities".

---

If multiple elements on the page have the same page transition tag, abandon the page transition managed by this with an InvalidStateException.

I think this step & similar steps intend to abort these steps too?

[khushalsagar]

Should it be clearer that these are private slots on SameDocumentTransition?

Other than finished, which is a public attribute, all others are mentioned as internal slots. Does that not imply these are private?

I think we need to be clearer on how the document is chosen, as it matters when one iframe calls another iframe's implementation of SameDocumentTransition.

Do you have a pointer to a similar spec text for this?

The current behaviour in the spec skips calling the callback, which prevents the DOM change from happening, which seems wrong given that transitions are an enhancement.

I don't think the fact that transitions are an enhancement is relevant since this is when the developer makes an error in using the API. There should be a feature check (for unsupported browsers) even before the prepare call is made.

Need a stronger definition of "on the page"

Would it suffice to say multiple elements or pseudo-elements in the same "Document". Since the idea is to check elements in the same DOM which can participate in this transition. Not sure what's the significance of saying "rendered" elements.

limit it to document-connected elements that are rendered

Can you think of a spec which needs to refer to the same concept? We can update the check for duplicate page-transition-tags with the same text.

Does the root automatically meet these conditions?

That's an excellent question. It doesn't look like it does if you try the following example:

<style>
  html {
    position:relative;
    top:100px;
    border:1px solid black;
    contain:layout;
  }
  .outer {
    width:100px;
    height:100px;
    background:blue;
    margin:100px;
  }
  .inner {
    width:50px;
    height:50px;
    background:green;
    position:fixed;
    top:10px;
  }
  body {
    margin:0px;
  }
</style>
<body onload="printstuff()">
  <div class="outer">
    <div class="inner">
    </div>
  </div>
</body>

For fixed position elements, the presence of contain:layout changes whether a fixed position element uses the initial containing block as its containing block or the root element. At the same I'm not sure if we need this for the root element. One reason to require it is so the element creates a stacking context but since we use the stacking context created for root + top layer for the root image, that's not an issue here.

The other was to ensure all positioned descendants move with the root during the animation (as they would've in the actual DOM). Again since we're including everything painted in the viewport for the root image, I don't think that matters here. @vmpstr for his thoughts too.

It feels like these elements should be filtered out rather than cause errors.

You mean pretend that the element wasn't tagged? That's fine too. In fact that's what we end up doing if a style change on incoming elements causes this to happen midway in the transition.

Shouldn't it be after step 16 when the rendering is actually updated?

We generate the pseudo elements with outgoing snapshots in the last rendering update presented to the user before the updates are suppressed. The paint timing will also include the time to paint these pseudo elements. So I wanted to ensure that this step is before the mark paint timing step.

More comments incoming...

[khushalsagar]

Maybe need to use a more formal type here. Eg, is it a style object, is it a struct? etc etc

Its a struct with the fields that we reference as being set in the algorithm. I can add a definition outside to basically declare the struct before its used in the algorithm?

Nit: This is like a callback, so I don't think "this" can be referenced.

Sorry I didn't follow. Create transition pseudo elements is an algorithm which uses fields from SameDocumentTransition. We do a similar pattern for a lot of other algorithms like: update transition DOM and animate a page transition.

Feels like we need a more formal definition for "Suppress rendering opportunities".

Agreed. I've added an issue to follow up on this. Also how to deal with input during this phase.

I think this step & similar steps intend to abort these steps too?

Sorry, didn't follow what "these" means here.

[jakearchibald]

Should it be clearer that these are private slots on SameDocumentTransition?

Other than finished, which is a public attribute, all others are mentioned as internal slots. Does that not imply these are private?

The text on https://tabatkins.github.io/specs/css-shared-element-transitions/#api says "Additionally the following structures exist". From the prose it isn't really clear what they exist on, or that they're private slots.

---

I think we need to be clearer on how the document is chosen, as it matters when one iframe calls another iframe's implementation of SameDocumentTransition.

Do you have a pointer to a similar spec text for this?

Web locks seems to do it right https://w3c.github.io/web-locks/#dom-lockmanager-request

---

The current behaviour in the spec skips calling the callback, which prevents the DOM change from happening, which seems wrong given that transitions are an enhancement.

I don't think the fact that transitions are an enhancement is relevant since this is when the developer makes an error in using the API. There should be a feature check (for unsupported browsers) even before the prepare call is made.

I think this needs more thinking #169

---

Need a stronger definition of "on the page"

Would it suffice to say multiple elements or pseudo-elements in the same "Document". Since the idea is to check elements in the same DOM which can participate in this transition. Not sure what's the significance of saying "rendered" elements.

I think we need to considered 'rendered' too #170

---

limit it to document-connected elements that are rendered

Can you think of a spec which needs to refer to the same concept? We can update the check for duplicate page-transition-tags with the same text.

Hmm, it feels like CSS animations have the same concept when it comes to creating animations, but I couldn't find any handle spec text there. I think we can just talk about elements connected to the document which are rendered.

---

Does the root automatically meet these conditions?

That's an excellent question. It doesn't look like it does if you try the following example:

Feels like we need some exception for root in the spec then?

---

It feels like these elements should be filtered out rather than cause errors.

You mean pretend that the element wasn't tagged? That's fine too. In fact that's what we end up doing if a style change on incoming elements causes this to happen midway in the transition.

I guess this depends on #170 too.

---

Shouldn't it be after step 16 when the rendering is actually updated?

We generate the pseudo elements with outgoing snapshots in the last rendering update presented to the user before the updates are suppressed. The paint timing will also include the time to paint these pseudo elements. So I wanted to ensure that this step is before the mark paint timing step.

Ohh, so the rendering that we use to capture the pseudos is different to step 16?

---

Maybe need to use a more formal type here. Eg, is it a style object, is it a struct? etc etc

Its a struct with the fields that we reference as being set in the algorithm. I can add a definition outside to basically declare the struct before its used in the algorithm?

It looks mostly ok now, although there are some reference to "outgoing data" which doesn't seem to exist https://tabatkins.github.io/specs/css-shared-element-transitions/#create-transition-pseudo-elements

---

Nit: This is like a callback, so I don't think "this" can be referenced.

Sorry I didn't follow. Create transition pseudo elements is an algorithm which uses fields from SameDocumentTransition. We do a similar pattern for a lot of other algorithms like: update transition DOM and animate a page transition.

Ignore me. It seems folks disagree on whether this pattern is ok, so let's just go with it. I've filed whatwg/webidl#1175 to get some clarity on it.

---

I think this step & similar steps intend to abort these steps too?

Sorry, didn't follow what "these" means here.

After this line:

If multiple elements or pseudo-elements on the Document have the same page transition tag, abandon the page transition managed by this with an InvalidStateException.

Is it the intention to proceed to the next line in the spec, even if the transition is abandoned? There's no explicit return/abort, so it will proceed to the next line.

[jakearchibald]

This has been addressed