[cssom-1] Replace steps of set a CSS declaration with some constraints #2924
Conversation
|
This would definitely need a working group resolution. |
cssom-1/Overview.bs
Outdated
| at an index after all of those <a>CSS declaration</a>s. | ||
|
|
||
| <li> | ||
| The steps must return true if the serialization of <var>declarations</var> is changed as a result of |
There was a problem hiding this comment.
Word this as "If the serialization of declarations was changed as result of these steps, then return true. Otherwise, return false."
There was a problem hiding this comment.
This wording "If ... then ... otherwise" sounds like part of steps, but this should be a constraint I think.
cssom-1/Overview.bs
Outdated
| or removed from <var>declarations</var>. | ||
|
|
||
| <li> | ||
| If there are <a>CSS declaration</a>s in <var>declarations</var> whose |
|
To adopt this change, we need to remove two existing wpt and introduce a looser wpt. I'm considering doing this in Gecko bug 1473180. It's not clear to me whether I can change the corresponding wpt without a resolution from the working group. |
|
@upsuper can you open an issue sketching the problem and the proposal and put the Agenda+ label? I'll attend this week's call and I can try getting a resolution there. |
|
I don't think we need a separate issue for that. I can put the problem and add Agenda+ on this PR directly. So in #1898 we resolved (again) that seting property should always append the new declaration to the end of the declaration block, which was added to spec in #2516. However, this change has a performance impact, because browsers would be required to trigger mutation observer, and flush style. (To be honest, flushing style may be avoidable, but it could be complicated to fix given how things are working nowadays.) Because of this, the change to Firefox has been put behind a pref and off by default, and the change to Chrome has been backed out. In the end, in #1898 what we really want is that physical properties and flow-relative properties can override each other correctly. There are possible algorithms to realize that without sacrificing performance in usual cases, but such algorithms may be non-trivial and it could become tricky to spec and have everyone agree on one. Thus, instead I propose that we just put down some basic requirements for this algorithm and allow implementations to explore. There would be observable behavior difference, but I don't think that would be too bad. If authors have been relying on the order, the original change itself would have been a breaking change. |
|
👍 |
|
The Working Group just discussed
The full IRC log of that discussion<dael> Topic: Replace steps of set a CSS declaration with some constraints<dael> github: https://github.com//pull/2924 <dael> ??: Issue is that the latest resolution on how set property behaved: it always appends to end of declaration so it's sane with logical prop it's a nightmare of webcompat and perf for Gecko and Blink <fantasai> dbaron, discussion at https://github.com//issues/1246 <dael> ??: WE turned it off in Gecko and backed out in block. xidorn had this proposal to let a se t of prop in a logicial group and in a UA dependent way that's in same logicial group it need to appear after so setProperty behaves correct <rego> s/??/emilio <dbaron> s/in block/in Blink/ <dael> emilio: I think frremy...what xidorn did in gecko which we haven't landed is that if you get to the case where a prop and there's another from the group that defers we append the new prop <dael> emilio: xidorn proposes to define in terms of constraints which I'm okay, but prefer define properly. Onlyr eason not to do is proposal from frremy. We need to decide if we're fine resolving like this or if fine to say it's constaints UA can do what they want or define algo in spec <dael> frremy: From what I recal my proposal was pretty in line with constaints. I'm fine with them as defined. Good to have UA experiment. If it's fine we can refine further. Fine to go with xidorn proposal for now. It makes a lot of sense. <dael> emilio: Okay <dael> astearns: I agree emilio it's good to have things properly defined once we have impl experience and can determine the constraints. Happy starting with the PR and adding <dael> emilio: People fine with gecko algo as an example? <dael> florian: SOunds okay <dael> astearns: As a note? <dael> emilio: Pretty much <AmeliaBR> present + <dael> astearns: Objections to having the PR https://github.com//pull/2924 as the start of a set of constraints with gecko algo as an exmplae in a note? <dael> frremy: Sounds good <dael> RESOLVED: havethe PR https://github.com//pull/2924 as the start of a set of constraints with gecko algorithm as an example in a note <dael> astearns: Anything else on this? |
|
Updated the PR to include an example algorithm that we are using in Gecko. Related web-platform test would be landed (by sync bot) as soon as the Gecko change gets merged. |
So what's the current behaviour? How interoperable are they? What would it take to converge behaviour in some other way to what we previously resolved in #1898? |
I think we should have that landed first.
Current behavior is to always update existing declaration with the same property name. I believe at least Blink and Gecko both do this, likely WebKit as well. Edge may have some other idea. Blink (and probably WebKit as well) doesn't trigger mutation observer if the value and flag aren't changed, while Gecko did. That caused performance issue for us and thus we fixed that in 62 or 63.
I don't think we are going to use the algorithm we previously resolved in #1898 because that has webcompat issue. We may eventually converge to some algorithm, but it's probably not going to happen very soon, as a conformant algorithm without webcompat issue is going to be somewhat complicated, and getting everyone agree on every details could take quite a bit of time. Also there are other stuff currently in my mind that we should probably add to this constraints which may make things more complicated... |
|
BTW, the corresponding web-platform test is going to be landed in web-platform-tests/wpt#12065. |
| Issue: Should we add something like "Any observable side effect must not be made outside | ||
| <var>declarations</var>"? The current constraints sound like a hole for undefined behavior. | ||
|
|
||
| Note: The steps of <a>set a CSS declaration</a> are not defined in this level of CSSOM. The user agent may |
There was a problem hiding this comment.
nit: plural: user agents may use different algorithms as long as the contraints above hold
(This depends on concepts being added to css-logical in #2922.)
This commit replaces the steps of "set a CSS declaration" with some constraints, so that user agents are allowed to sort the declarations in whatever way they want as far as those constraints hold.
It's not clear to me what is the right wording for this.