Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cssom] Overlapping step 5 and 6 in insert a CSS rule #8654

Closed
cdoublev opened this issue Mar 28, 2023 · 7 comments
Closed

[cssom] Overlapping step 5 and 6 in insert a CSS rule #8654

cdoublev opened this issue Mar 28, 2023 · 7 comments

Comments

@cdoublev
Copy link
Collaborator

I think step 6 of insert a CSS rule can never throw...

  1. If new rule cannot be inserted into list at the zero-index position index due to constraints specified by CSS, then throw a HierarchyRequestError exception.
  2. If new rule is an @namespace at-rule, and list contains anything other than @import at-rules, and @namespace at-rules, throw an InvalidStateError exception.

... because CSS Namespaces 3 defines this constrainst (detected in step 5):

Any @namespace rules must follow all @charset and @import rules and precede all other non-ignored at-rules and style rules in a style sheet.

However CSS Namespaces 3 does not explicitly define whether a non-conforming @namespace must be invalid or if it must be ignored.

<style>
  style {}
  @namespace ns "http://ns.org";
</style>
<script>
  try {
    document.styleSheets[0].insertRule('@namespace ns "http://ns.org";', 1)
  } catch (error) {
    // Chome: InvalidStateError
    //    FF: HierarchyRequestError
    console.log(error.name) 
  }
</script>

If it must be invalid and I am not mistaken, switching step 5 and 6 could be an alternative to removing step 6.

@cdoublev
Copy link
Collaborator Author

I am sorry, InvalidStateError would be thrown in the following case:

const sheet = new CSSStyleSheet()
sheet.insertRule('style {}')
sheet.insertRule('@namespace ns "http://ns.org";') // InvalidStateError

I guess I miss an hour of sleep...

@tabatkins
Copy link
Member

Right, step 6 isn't detecting positional errors, it's detecting a "potentially changes the interpretation of all other rules" action and disallowing it. There's nothing wrong with the stylesheet that Step 6 throws on in theory, but in practice it means a change to the entire sheet datastructure, so it's not a localized change.

@cdoublev
Copy link
Collaborator Author

cdoublev commented Apr 4, 2023

Would you be kind enough to also explain me how the interpretation of a single @layer statement, @font-face, @media, etc, would change after inserting @namespace, please?

@tabatkins
Copy link
Member

They don't. But carefully dividing CSS syntax into "things that get affected by a namespace change" and "things that don't", and then requiring engines to verify that the stylesheet contains only the "not affected" rules, and keeping that division up-to-date as we add new rules, is all in all not a worthwhile use of anyone's time. The simple rule as it exists does the job sufficiently well.

@cdoublev
Copy link
Collaborator Author

Ok, thanks. I have no problem with this position but I needed it clarified.

@cdoublev
Copy link
Collaborator Author

cdoublev commented Apr 18, 2023

Actually I do not see how it makes it less complex for engines. An author could write this style sheet:

@layer base;
@namespace ns "https://ns.org";

But it could not construct the corresponding style sheet?

const sheet = new CSSStyleSheet()
sheet.insertRule('@layer base;')
sheet.insertRule('@namespace ns "https://ns.org";', 1) // InvalidStateError

It seems to boil down to sharing the same validation function...

function isInvalidStateForNamespace(rule, rules) {
  return rules.some(isNotImportOrNamespaceOrLayerStatement)
}
function parseStyleSheet(rules) {
  const valid = []
  for (const rule of rules) {
    if (isNamespace(rule) && isInvalidStateForNamespace(rule, valid)) {
      continue
    }
    // ...
    valid.push(rule)
  }
  return rules
}
function insertCSSRule(rule, list) {
  // ...
  // Nope, CSSOM requires a different validation here!
  if (isNamespace(rule) && isInvalidStateForNamespace(rule, list)) {
    throw InvalidStateError
  }
  // ...
}

@tabatkins
Copy link
Member

Correct: they could write such a sheet, and they could not construct such a sheet.

Again, this is because making and keeping the OM rule "correct" simply isn't worth anyone's time. To a first approximation, no one constructs and inserts @namespace rules. (To a first approximation no one uses @namespace at all.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants