Setting element border style loses some of the border values

[dpvc]

If you set el.style.border = "1px solid back", then el.style.border will be set to black, but the 1px and solid are lost. It appears that only the last word in the string is used, so el.style.border = "1px solid" sets it to solid.

Here is a test case:

var jsdom = require("jsdom").jsdom;
var doc = jsdom("");
var el = doc.createElement("div");
el.style.border = "1px solid black";
console.log(el.style.border);          // prints "black"
console.log(el.style.cssText);         // prints "border: black"

This is with jsdom 6.5.1.

[domenic]

/cc @chad3814

[pkra]

Is there a chance on an update (or suggestion) on this? FWIW, still seeing this on jsdom 7.1.0.

[domenic]

The best thing to do would be to submit a PR to https://github.com/chad3814/CSSStyleDeclaration fixing the issue.

[pkra]

Thanks for the pointer!

[dpvc]

I spent quite some time looking into this, and it turns out that the issue is not with CSSStyleDeclaration itself, but in how jsdom uses it. The problem seems to be the on-change handler in level2/html.js. If you remove the callback, then the example code above produces the correct results.

It appears that these lines are designed to get the updated style definition added to the element as the style attribute, and indeed if you remove the callback from html.js and add console.log(div.outerHTML) to the end of the example above, the result is <div></div> with no style attribute.

Although the situation is pretty complicated, it seems that what is happening is that during the that.setAttribute('style', newCssText) call, the style attribute is being modified for each of the individual style attributes from the border (i.e., the style is set for border-width:1px, border-style:solid, and border-color:black separately). This causes the previous settings to be lost, and so only the last one is being retained.

My suggested fix is to replace those three lines with

      if (!that._settingCssText) {
        that._settingCssText = true;
        that.setAttribute('style', newCssText);
        that._settingCssText = false;
      }

so that recursive calls to setAttribute() aren't made. This seems to resolve the issue for me. Of course, I don't know the codebase for jsdom very intimately, so am not sure if that will cause any other problems. But the change doesn't seem to have caused any problems during my testing of the change in MathJax-node, which can do a lot of style setting.

If you want me to make a pull request for this, I will. Let me know.

[domenic]

Thanks so much for looking into this! Right now we have a major refactor that will touch that file, so hold off on a pull request for a bit :). But after that lands (#1333) I'd love a pull request that fixes that and leaves all other tests passing.

[dpvc]

OK, will do. I've subscribed to 1333 and will do a PR after that has been merged (provided my fix still works) :-)