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.

Sign up for GitHub

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

Jump to bottom

[cssom] Consider removing the ability to insert / remove @namespace rules from the OM. #2716

Open
emilio opened this issue May 28, 2018 · 6 comments
Open

[cssom] Consider removing the ability to insert / remove @namespace rules from the OM. #2716

emilio opened this issue May 28, 2018 · 6 comments
Labels
cssom-1 Current Work

Comments

@emilio
Copy link
Collaborator

emilio commented May 28, 2018

I was fixing an edge case in Firefox's CSSOM implementation (https://bugzilla.mozilla.org/show_bug.cgi?id=1464865) when I started realizing of all the different bugs that our implementation had.

Well, turns out most of the other implementations also have them, for example, even this simple test-case:

<!doctype html>
<style id="s">
</style>
<div>Should not be green</div>
<script>
  // Insert a namespace rule, remove it.
  s.sheet.insertRule('@namespace myhtml url("http://www.w3.org/1999/xhtml")', 0);
  s.sheet.deleteRule(0);
  s.sheet.insertRule('myhtml|div { color: green }', 0); // Should not be green.
</script>

Is wrong and shows green in every single browser. I don't even want to start thinking of preserving the order when the namespace string is the same and such. Nobody removes from the map either from my testing.

The reason this happens is because everyone to the best of my knowledge represents namespaces as a single hash map, and dealing with changes to it is slightly painful.

These bugs can of course be fixed, but I'm not aware of anyone trying to improve the situation here, and personally having the namespace map immutable at parse time would be really nice.

Is there anyone planning to fix implementations here? Or would it be reasonable to consider trying to drop deletion / insertion of namespace rules via the OM?

cc @lilles @FremyCompany @zcorpan
@emilio emilio added the cssom-1 Current Work label May 28, 2018
@lilles
Copy link
Member

lilles commented May 29, 2018

So, one problematic issue is:

"A type selector or universal selector containing a namespace prefix that has not been previously declared is an invalid selector."

which means the whole rule will be dropped at parse-time. Reviving that rule if a @namespace rule is inserted does not sound feasible in Blink, nor makes sense spec-wise. Also, what should happen if you remove a @namespace rule? Should the style rules using that prefix be removed?

I think it makes sense to not allow @namespace rule mutations.

@emilio
Copy link
Collaborator Author

emilio commented May 29, 2018

So that complexity is already worked around in the spec in:

https://drafts.csswg.org/cssom/#insert-a-css-rule

Where step 6 says:

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.

But given we're bogus even with that rule accounted for, and that that restriction is already pretty limiting, I think it makes sense to just disallow it completely.

@tabatkins
Copy link
Member

How do you construct a stylesheet with a namespace rule in it, then?

@emilio
Copy link
Collaborator Author

emilio commented May 31, 2018

cssText or such I guess. You can then append other style rules at your will.

@tabatkins
Copy link
Member

Stylesheets don't have a .cssText tho. ^_^

@emilio
Copy link
Collaborator Author

emilio commented May 31, 2018

Whoops, never write from memory. textContent and similar on a <style> element works and creates a new sheet, but yeah, agree it's uglier...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
cssom-1 Current Work
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@tabatkins @lilles @emilio