- From: CSS Meeting Bot via GitHub <sysbot+gh@w3.org>
- Date: Wed, 31 Jul 2024 16:37:25 +0000
- To: public-css-archive@w3.org
The CSS Working Group just discussed `[cssom][css-nesting] Error behavior for insertRule with declaration blocks.`, and agreed to the following:
* `RESOLUTION: throw on insertRule only when there are no valid declarations. Do not throw otherwise`
* `RESOLVED: throw on insertRule only when there are no valid declarations. Do not throw otherwise`
<details><summary>The full IRC log of that discussion</summary>
<noamr> emilio: we resolved to be able to call insertRule for various declarations, but we didn't resolve on error handling<br>
<noamr> emilio: 2 options: (1) do nothing, effectively making this API not throw at all, right now we throw. (2) throw, but only when we get empty declaration blocks<br>
<noamr> emilio: 2 is a bit weird if you only specify invalid declarations, but would be consistent with how it's parsing. Seems like there's a variety of opinions on the thread or other options<br>
<noamr> astearns: other people who have commented<br>
<noamr> Mattieud: we thought that maybe we'd add another function, insertDeclaration, to avoid this issue.<br>
<emilio> q+<br>
<noamr> MattieuD: This was one of the options we resolved on last time, still option for the discussion.<br>
<astearns> ack emilio<br>
<noamr> emilio: I thought we wanted insertRule work and would do the right thing<br>
<noamr> TabAtkins: the resolution was to prefer insertRule, but fall back if necessary due to web compat<br>
<noamr> emilio: were you against throwing on declaration blocks?<br>
<noamr> TabAtkins: I want to be consistent. Need to look at existing implementation. If we throw now we should keep throwing<br>
<noamr> emilio: insertRule current throws<br>
<noamr> TabAtkins: then insertRule should throw if the declaration is consisting only of invalid rules<br>
<noamr> emilio: that's the current spec<br>
<noamr> matthieud: The spec says to throw syntax errors in multiple places<br>
<noamr> astearns: matthieud are you ok with matching the throwing behavior here, or prefer a separate method?<br>
<noamr> matthieud: current is OK, it's a bit weird and surprising when mixing valid/invalid declarations, but that's fine<br>
<astearns> ack fantasai<br>
<noamr> fantasai: throwing on "only invalid rules" vs throwing on "mixed" is weird and inconsistent. we should throw on "some invalid stuff"<br>
<emilio> q+<br>
<astearns> ack emilio<br>
<noamr> TabAtkins: we can't throw on "some invalid" because of forward compatibility, as people won't be able to use it for multiple browsers in the future as it would always throw<br>
<noamr> fantasai: it would be weird if something was throwing for not having any valid rules and then suddenly not throw when one valid rule added<br>
<TabAtkins> we currently throw on "one invalid at-rule", I'm not sure that throwing on "one invalid decl" is inconsistent<br>
<noamr> emilio: that's exactly the CSS parser behavior<br>
<florian> q+<br>
<matthieud> matthieud: the CSS parser and the CSSOM have different behavior "on purpose" : the CSS parser happily skip invalid rules/at-rules/declarations - but the CSSOM throws on invalid rule<br>
<noamr> emilio: It's not that bad, because that's how the CSS parser behavior works<br>
<noamr> emilio: I don't recall the behavior when there are multiple rules<br>
<fantasai> s/any valid rules/any valid declarations/<br>
<fantasai> s/one valid rule/one valid declaration/<br>
<noamr> Matthieud: you can't insert multiple rules with insertRule<br>
<astearns> ack florian<br>
<noamr> florian: from my POV if we throw on everything that's invalid or empty and don't throw on mix valid/invalid, than that's consistent<br>
<noamr> florian: you need at least 1 valid declaration to be valid<br>
<emilio> q+<br>
<noamr> fantasai: if we care about the idea of forward compat, if we use 3 new syntax and one old syntax, and then you remove the old one it starts throwing, it doesn't make sense<br>
<noamr> fantasai: if we want to throw invalid declarations we should always do that<br>
<noamr> florian: it works the same as with inserting an empty string<br>
<astearns> ack emilio<br>
<noamr> emilio: I don't disagree with fantasai but I don't think it's possible because it would make the API never throw with any input<br>
<TabAtkins> `insertRule("@invalid {...}")` throws - this is perhaps a bad idea for forwards compat *as well*, but it's the current long-standing behavior<br>
<noamr> emilio: that's a pretty significant breaking change<br>
<noamr> emilio: the current spec is perhaps the most sensible solution<br>
<astearns> ack fantasai<br>
<noamr> florian: I argue that it makes conceptual sense, not necessarily that it's convenient for devs<br>
<noamr> fantasai: I agree we shouldn't change the behavior for @ rules, that's fine since it's one rule at a time<br>
<noamr> fantasai: I'm arguing that for invalid declarations only, or a mix of valid and invalid declarations<br>
<emilio> q+<br>
<matthieud> q+<br>
<noamr> fantasai: as a consequence we need to decide whether we throw on all-invalid and empty string<br>
<astearns> ack emilio<br>
<fantasai> s/that for/for consistency on/<br>
<noamr> fantasai: we need to either throw or not throw on all-invalid/non-invalid/empty-string<br>
<noamr> fantasai: the 3 of them should behave in the same way<br>
<fantasai> s/non-invalid/some-invalid/<br>
<noamr> emilio: not-throwing on all invalid is not feasible, because you don't know if you were given invalid declarations or an invalid at rule. it gets weird<br>
<florian> q+<br>
<noamr> florian: right, it means changing the existing situation where we throw on invalid rule<br>
<noamr> s<br>
<noamr> s/florian/Matthieud<br>
<fantasai> s/as a consequence we need to decide/and I think we should be consistent about/<br>
<noamr> emilio: then we should other do what's on the spec, or throw on some invalid, but it's a bit odd how the rest of CSS works<br>
<astearns> ack matthieud<br>
<TabAtkins> At the moment there's no way, given the parsing algos, to tell the difference between "empty string" and "all invalid decls"; invalid decls are dropped during parsing.<br>
<noamr> MatthieuD: this would be a breaking API change<br>
<astearns> ack florian<br>
<TabAtkins> If we try to just split on "empty string" vs "not empty", then a string containing just an invalid rule will pass (whereas currently it throws).<br>
<noamr> florian: maybe I'm failing to understand something, but the way I see it, you want to throw as a reaction to anything invalid being passed, while I think we should throw on the absence of something valid<br>
<TabAtkins> We'd had to instead do some heuristic on the stuff in the string to see if it *might* look like there's a decl in there.<br>
<noamr> florian: throwing is not a signal of "something was wrong" but rather "there was not a single thing that was right"<br>
<fantasai> Could check for initial "@" for "invalid at-rule". Probably that's what most people would rely on.<br>
<noamr> florian: at least it's useful in some cases where you insert some valid rules and some invalid<br>
<TabAtkins> Can't look for just an @; style rules are allowed too<br>
<noamr> fantasai: it's ok conceptually, do we want to be resilient on wrong declarations, if we don't throw on most implementations and silently handle invalid declarations, and then some implementation arbitrarily succeeds<br>
<fantasai> TabAtkins, ahh right<br>
<noamr> florian: isn't that useful, when you want to get told when "one thing works" vs "nothing works"<br>
<TabAtkins> Grammar is hard (this is why I wanted insertDeclarations(), fwiw)<br>
<matthieud> I don't think trying to find an heuristic like looking for an @ is a good idea (and it already doesnt work for nested style rule)<br>
<emilio> q+<br>
<noamr> fantasai: why do I care if I dropped 2/3 vs 3/3?<br>
<noamr> florian: it depends why you inserted more than one<br>
<noamr> florian: maybe you added a prefixed and a non-prefixed, it depends on why you use the API<br>
<noamr> florian: this works for fallback properties, where you want "at least one of these to work"<br>
<noamr> florian: you called insertRule, if it succeeded you probably meant it because you have enough fallbacks, but if nothing goes through something went wrong<br>
<noamr> fantasai: this might not be more or less wrong<br>
<astearns> ack emilio<br>
<noamr> emilio: it consistent with how the API behaves right now. If the rule list grows by 1 it doesn't throw, otherwise it throws<br>
<noamr> emilio: it's weird because it's inserting something that's not a rule into a rule list<br>
<noamr> astearns: a lot of comments on IRC, not entirely sure we're ready for a resolution. take back to the issue?<br>
<noamr> fantasai: I'm not going to object, people will be confused, but I don't know if there is a better option<br>
<noamr> fantasai: given the restrictions, I don't see a better solution<br>
<noamr> astearns: concerns about "only throwing when there is nothing valid?"<br>
<noamr> proposed resolution: throw for insertRule if there are no valid declarations, not throw otherwise<br>
<noamr> RESOLUTION: throw on insertRule only when there are no valid declarations. Do not throw otherwise<br>
<noamr> RESOLVED: throw on insertRule only when there are no valid declarations. Do not throw otherwise<br>
</details>
--
GitHub Notification of comment by css-meeting-bot
Please view or discuss this issue at https://github.com/w3c/csswg-drafts/issues/10520#issuecomment-2260929551 using your GitHub account
--
Sent via github-notify-ml as configured in https://github.com/w3c/github-notify-ml-config
Received on Wednesday, 31 July 2024 16:37:26 UTC