WICG / construct-stylesheets

API for constructing CSS stylesheet objects
Other
138 stars 25 forks source link

Sheet's rules after replace() fails to load @import rule #122

Closed nordzilla closed 4 years ago

nordzilla commented 4 years ago

In PR #112 I modified the spec description for replace() to only return a promise instead of throwing, as noted in the TAG API guidelines (here). I believe that I accidentally changed the semantics in the case of a failed @import rule.


Intended Behavior

I believe that the intended behavior should be that if an @import rule fails to load, the promise is rejected with a network error, but the sheet's rules are still cleared.

Actual Behavior

The new text that I wrote in #112 would suggest that if the @import rule fails to load, the sheet still contains the rules it had previously.


I want to double check that the intended behavior is correct. If so, I will create a PR to amend the mistake that I made.

Note that step 3 in the old text clears the sheets rules. I believe that I inadvertently removed this action in what is now step 4 of the new text.

Here are the rules before the change:

image

Here are the rules after the change:

image


cc @tabatkins @rakina

rakina commented 4 years ago

Yeah I think we should not replace the rules in that case. The behavior will be consistent with replaceSync, where there will be no change if at any point there's an error https://wicg.github.io/construct-stylesheets/#dom-cssstylesheet-replacesync

nordzilla commented 4 years ago

Closing due to the decision to deprecate replace(): https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/RKG8oxp22RY/fdFnG1rGCgAJ

mfreed7 commented 4 years ago

We’re not deprecating replace() entirely, just the case where the replacement sheet contains an @import rule. Calls to replace() without @import will continue working as is.

nordzilla commented 4 years ago

Thank you for this update. But I am going to leave this issue as closed, because it's more-or-less a subset of #123