Skip to content

Conversation

@dpvc
Copy link
Member

@dpvc dpvc commented Jan 2, 2026

This PR fixes the handling of some border CSS properties like border-color that incorrectly ended up being translated to border specifications when they shouldn't. Its also updates the tests to check this.

The test file adds a test for setting border-color, which was the issue reported. It also moves the background check out of the border section and into a new border test section.

The ts/util/Styles.ts file is modified as follows:

The connection data adds a new parts array to list the sub-properties like border-color that modify other styles like border-top-color but that don't combine directly to the border property. It also adds a subPart boolean to indicate a property like border-color that acts in that way. Previously, we used combine: null to indicate this, but that was not a very clear indication.

A new combinePart() function is used for border-color and the other ones like it to handle the adjustments to the styles for those. This uses combineTRBL() to first change the properties like border-top-color, then calls combineChildren() to see if a combined border property can now be used, then calls combineSame() to see if border-color can be used in place of border-top-color, etc., and finally calls the new combineParent() function described below.

The combineWSC() function is modified to only combine only if there is more than one of the three properties that is specified, so border-color: red alone won't move to border: red.

The connection definitions are modified to mark border as having width, style, and color sub-parts, and the definitions for those now use combine: combinePart and subPart: true.

The cssText output now checks whether a combined part property is available before including the more specific ones, so that border-top-color will not be included if border-color is available.

The set() function is refactored to move the code that handles combining changes into parent properties into a separate combineParent() function (since it is now needed in several places). It also now uses the subPart connection property rather than combine === null to test for border-color and similar properties, and in then calls the connection's combine() function, which now points to combinePart() for these border properties. Finally, after combining parents, if the property is a specific sub-part (like border-top-color), now the it is changed, we check to see if the other TRBL parts can be combined (e.g., as border-color).

The code for combineParent() is taken from the old set() definition, and simplified a bit (caching the Styles.connect[name] locally as connect), and then adds code to remove sub-parts when the child properties have been combined (so if border is able to be used, the border-color and other sub-parts will not be needed).

The rest of the changes are simplifications or comment clean-ups.

Resolves issue mathjax/MathJax#3490.

@dpvc dpvc requested a review from zorkow January 2, 2026 15:22
@dpvc dpvc added this to the v4.1.1 milestone Jan 2, 2026
@codecov
Copy link

codecov bot commented Jan 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.51%. Comparing base (82cabf4) to head (481e9d7).
⚠️ Report is 18 commits behind head on develop.

Additional details and impacted files
@@            Coverage Diff             @@
##           develop    #1408     +/-   ##
==========================================
  Coverage    86.50%   86.51%             
==========================================
  Files          340      340             
  Lines        85952    85987     +35     
  Branches      4816     3184   -1632     
==========================================
+ Hits         74357    74392     +35     
- Misses       11572    11595     +23     
+ Partials        23        0     -23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dpvc dpvc changed the title Update handling of border parts like border-color and add tests for that. (mathjax/MathJax#3490) Update handling of CSS border parts like border-color and add tests for that. (mathjax/MathJax#3490) Jan 2, 2026
Copy link
Member

@zorkow zorkow left a comment

Choose a reason for hiding this comment

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

A few small things.

}
}
if (parts.length) {
if (parts.length > 1) {
Copy link
Member

Choose a reason for hiding this comment

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

Why is the style deleted if it has only one component?

Copy link
Member Author

Choose a reason for hiding this comment

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

When there is only one part, it will be handled by a subpart. That is, if border soul be set to red, it is better handled as border-color: red. That was the the basis of the original issue, as border: red also resets the style and width, while border-color: red does not.

I'm not sure whether this should really be parts.length === 3 instead.

Copy link
Member

Choose a reason for hiding this comment

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

Should be fine then.

@dpvc dpvc requested a review from zorkow January 7, 2026 12:09
@dpvc
Copy link
Member Author

dpvc commented Jan 7, 2026

I've made the changes; can you re-review?

@dpvc dpvc merged commit a9f3bf1 into develop Jan 7, 2026
3 checks passed
@dpvc dpvc deleted the issue3490 branch January 7, 2026 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants