Skip to content

Prepare CSSStyleDeclaration-impl.js and CSSStyleProperties-impl.js#3957

Closed
asamuzaK wants to merge 25 commits intojsdom:mainfrom
asamuzaK:style2
Closed

Prepare CSSStyleDeclaration-impl.js and CSSStyleProperties-impl.js#3957
asamuzaK wants to merge 25 commits intojsdom:mainfrom
asamuzaK:style2

Conversation

Copy link
Copy Markdown
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this. The overall approach looks very promising.

"Inherited system color keyword is observable on caret-color": [fail, computedStyleMap not implemented]
"Inherited system color keyword is observable on fill": [fail, computedStyleMap not implemented]
"Inherited system color keyword is observable on stroke": [fail, computedStyleMap not implemented]
"System color doesn't compute to itself on box-shadow": [fail, Need cssstyle fix]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do these regress?

Copy link
Copy Markdown
Contributor Author

@asamuzaK asamuzaK Oct 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is because cssstyle does not yet properly implement parsers for box-shadow and other color related properties.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But why did they pass before, and fail with this PR?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't looked into the details, but I suspect that this may have coincidentally matched a color conversion table that jsdom had locally before.
My personal opinion is that it's expected to fail at this point.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found the reason.
For example, in the case of box-shadow, the propertyList.js contains:

[
  "box-shadow",
  {
    "href": "https://drafts.csswg.org/css-borders-4/#propdef-box-shadow",
    "initial": "none",
    "inherited": "no",
    "computedValue": "see individual properties",
    "name": "box-shadow",
    "styleDeclaration": [
      "box-shadow",
      "boxShadow"
    ]
  }
],

"computedValue" is set to "see individual properties" and not to "a/the computed color"

See lib/jsdom/living/helpers/style-rules.js#161

@asamuzaK asamuzaK force-pushed the style2 branch 2 times, most recently from da91718 to fe6cb7d Compare November 2, 2025 11:50
}

throw new TypeError(`Internal error: unrecognized computed value instruction '${computedValue}'`);
function setDeclarationForElement(elementImpl, opt, initialize = false) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the function

@asamuzaK asamuzaK force-pushed the style2 branch 2 times, most recently from de1ccd8 to e7f4332 Compare November 13, 2025 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment