Fix/fail parse base64 in style attr

[cupojoe]

Fixes issue #11

[wooorm]

This will work for url(), but it would still not work for something like content: ';'; color: red, or comments, right?

As this whole string parsing only happens if style is a string, you could:

1. if you’re in control over the nodes, use style as an object
2. if someone else creates nodes, traverse the tree and transform styles with css-declarations to properly parse strings into objects?

[cupojoe]

@wooorm I'm curious why aren't you using css-declarations to begin with?
I'm looking into it now.

[wooorm]

Because it’s has a big footprint in file-size: it contains a full blown CSS parser!

[cupojoe]

Because it’s has a big footprint in file-size: it contains a full blown CSS parser!

It seems like the perfect tool for the job. I just tried it and it works flawlessly. I understand the footprint portion, but it would seem that if you need to parse CSS, a full parser is needed.

This will work for url(), but it would still not work for something like content: ';'; color: red, or comments, right?

Alternatively, I could add a case for content properties similar to what I'm doing with url() values.

if you’re in control over the nodes, use style as an object

Yeah, that is the case.

if someone else creates nodes, traverse the tree and transform styles with css-declarations to properly parse strings into objects?

I can definitely traverse the tree and convert style strings into objects, but that would be solving just my use case. Wouldn't it be better if we can at least consider the use cases where this fails for people with no control over the tree?

[wooorm]

It seems like the perfect tool for the job. I just tried it and it works flawlessly. I understand the footprint portion, but it would seem that if you need to parse CSS, a full parser is needed.

But that’s not a viable options in the browser

Alternatively, I could add a case for content properties similar to what I'm doing with url() values.

Doesn’t account for comments

if you’re in control over the nodes, use style as an object
Yeah, that is the case.

In that case you can write style as an object!

I can definitely traverse the tree and convert style strings into objects, but that would be solving just my use case. Wouldn't it be better if we can at least consider the use cases where this fails for people with no control over the tree?

We could create a utility that does it, so other people can benefit from this?

[cupojoe]

Do you think there is value in trying to list out the edge cases and try to patch the parser in the same way this PR is doing or should we call it a day and close this one?

[wooorm]

I don’t think it’s possible to fix all the potential problems like this.

• I’d rather not support this in this package, so this package can stay rather small
• I get that others want proper support, but I’d suggest using a) objects or b) another project that contains a full CSS parser!

From the top of my head, a project like that could look as follows:

var parse = require('css-declarations').parse;
var visit = require('unist-util-visit');

module.exports = transform;

function transform(tree) {
  visit(tree, 'element', visitor);
}

function visitor(node) {
  var props = node.properties;
  if (typeof props.style === 'string') {
    props.style = parse(props.style);
  }
}

[cupojoe]

Closing this in favor of transforming string style attributes before passing the tree to hast-to-hyperscript to keep this library slim.
Closes #11