w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.38k stars 643 forks source link

[css-variables] CSS wide keywords in custom properties with dependencies #9131

Open emilio opened 12 months ago

emilio commented 12 months ago

I was investigating a Firefox bug about custom properties with css-wide keyword fallbacks.

The following test-case (live) renders differently in Chromium and Safari. I can explain the difference, I believe.

Safari seems to first figure out what declarations apply, then you resolve dependencies between the values, and treat revert-layer as invalid at computed value time. This is similar to how longhands work.

Chromium seems to do something different, where their dependency-cycle detection also seems to deal with revert-layer somehow.

To me Safari's behavior seems easier to understand, but I might be biased because that's how Gecko implements custom-properties too. I think Chromium's behavior is more correct tho, but a clarification on the spec and sanity-checking my understanding of the situation would be great.

@mdubet / @andruud is my understanding above right?

emilio commented 12 months ago

I think https://drafts.csswg.org/css-variables/#cycles means that what Gecko and WebKit are implementing seems a bit closer to what the spec says, I think?

The spec assumes one value per property, which is true if you do that... But then again the behavior difference between --foo: var(--non-existing, unset) and --foo: unset seems odd.

Loirooriol commented 12 months ago

I'm confused, you say Gecko behaves like WebKit, but I'm getting purple on both Gecko and Blink, while WebKit is blue.

emilio commented 12 months ago

Gecko behaves like WebKit as in "how we deal with custom properties". We didn't honor wide keywords from fallback on custom properties (there's a patch in the bug linked above to fix that).

andruud commented 12 months ago

The following test-case (live) [...]

You seem to be worried about cycle detection, but I don't see a cycle in that test? Maybe I'm just blind.

emilio commented 12 months ago

Yeah, I agree there's no cycle per se in that test, no, but revert-layer might introduce another edge in the graph, effectively.

The main difference in implementation is, I think, that Gecko/WebKit perform the cycle detection during substitution, after we've computed the un-substituted value of all properties. Substitution can't (easily at least, given how we implement these) introduce new edges, because we've lost the information of the "potentially-reverted-to" declarations.

Loirooriol commented 12 months ago

I still think this is totally unrelated to cycles.

<!doctype html>
<style>
  .outer {  --color: red }
  @layer {
    .inner {  --color: green  }
  }
  .inner {  --color: revert-layer;  color: var(--color)  }
</style>
<div class="outer">
  <div class="inner">
      Which color am I?
  </div>
</div>

Gecko & Blink: green WebKit: red

andruud commented 12 months ago

@Loirooriol Yeah, that may be.

Perhaps @emilio is concerned about a case like this, where a revert appears "late" and adds a dependency that's not otherwise there in the un-reverted value:

<!doctype html>
<style>
  .outer { --x: blue; color: red;}
  @layer {
    .inner {
      --x: var(--y);
    }
  }
  .inner {
    /* --x and --y are cyclic via revert-layer */
    --x: var(--unknown, revert-layer);
    --y: var(--x, yellow);
    color: var(--y, green);
  }
</style>
<div class="outer">
  <div class="inner">
      Which color am I?
  </div>
</div>
kizu commented 3 months ago

I was playing a bit with the revert-layer in custom properties, and I did find that there is a possibility to introduce a cyclic dependency via layers as @andruud shows.

And there are for sure cases where we'd want the --foo: var(--bar, revert-layer) to work, I did attempt this a few times when doing experiments for my latest article that utilizes cyclic dependencies.

I think I like how Chrome handles this the most currently: regardless of if a property is registered, I'd want the revert-layer in --foo: var(--bar, revert-layer) to revert the --foo.

When registering the property, it seems that in Safari we can revert-layer a custom property, but only if it is registered with some specific syntax (not universal).

In Firefox Nightly, right now, we can't revert-layer regardless if we register property or not.

Here is a CodePen with an example: https://codepen.io/kizu/pen/MWRGoNp

(I think, ideally it would be nice to have some way to pass around and apply the CSS-wide keywords through custom properties, but that's probably a bigger, separate issue)

tabatkins commented 3 months ago

Chrome's behavior, as far as I can tell, is what I'd naively expect to happen, particularly in Ander's latest example where Chrome gives green. --x and --y are initially not cyclic, but fallback causes --x to change to a value that is cyclic with --y, so the final var(--y, green) triggers fallback too.