Handle global keywords in CSS shorthand property handlers#4117
Merged
Conversation
Several shorthand property handlers (border, background, and their sub-shorthands) crashed or produced incorrect results when encountering CSS-wide keywords like "inherit" or "initial". The root cause is that `parse()` functions return strings for global keywords, but many callers assumed object/array returns. - In `processBorderProperties`, handle global keywords uniformly at the top of the loop before any property-specific branching, fixing `cssText = "border-top: inherit"` (and all border side/line shorthands) which previously threw a TypeError swallowed by the cssText setter's catch block. - In `matchesBorderShorthandValue`, return false for non-object parse results, fixing `border: inherit` followed by setting a longhand like `borderTopWidth`. - In `prepareBorderStringValue` and `prepareBorderObjectValue`, guard `replacePositionValue` calls against global keyword inputs, fixing `border: inherit` followed by setting a sub-shorthand like `borderTop`. - In `background.parse()`, return global keywords as strings (matching `border.parse()` behavior). Update the descriptor setter to propagate them to all longhands and the getter to verify longhand consistency before returning. - In `prepareProperties`, skip `replaceBackgroundShorthand` for global keyword shorthands, and fix a pre-existing Map mutation-during-iteration bug by updating `parsedProperties` instead of `properties`. - Mark `cssom-setProperty-shorthand.html` as passing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
LGTM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Several shorthand property handlers (border, background, and their sub-shorthands) crashed or produced incorrect results when encountering CSS-wide keywords like "inherit" or "initial". The root cause is that
parse()functions return strings for global keywords, but many callers assumed object/array returns.processBorderProperties, handle global keywords uniformly at the top of the loop before any property-specific branching, fixingcssText = "border-top: inherit"(and all border side/line shorthands) which previously threw a TypeError swallowed by the cssText setter's catch block.matchesBorderShorthandValue, return false for non-object parse results, fixingborder: inheritfollowed by setting a longhand likeborderTopWidth.prepareBorderStringValueandprepareBorderObjectValue, guardreplacePositionValuecalls against global keyword inputs, fixingborder: inheritfollowed by setting a sub-shorthand likeborderTop.background.parse(), return global keywords as strings (matchingborder.parse()behavior). Update the descriptor setter to propagate them to all longhands and the getter to verify longhand consistency before returning.prepareProperties, skipreplaceBackgroundShorthandfor global keyword shorthands, and fix a pre-existing Map mutation-during-iteration bug by updatingparsedPropertiesinstead ofproperties.cssom-setProperty-shorthand.htmlas passing.@asamuzaK after merging d589a8e I asked Claude if it could find any similar issues and this is what it found. WDYT?