From 7efcbb1142580560d5a5b0797ba41567a0682ef6 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 13 Oct 2026 10:46:34 +0000 Subject: [PATCH] css: stop rewriting valid positions into ones browsers reject `Position::to_css` prints the position of `background-position`, of the `background` and `mask` shorthands, and of radial and conic gradients. It wrote the three-value form for a side keyword that has no offset. `radial-gradient(circle at right 40%, red, blue)` printed `at right top 40%`, which only `background-position` accepts. The first three arms now test the offset, as lightningcss does. The fallback prints a length that is not zero as a length, so merged x and y longhands no longer print `10px top 5px`. Two arms from lightningcss commit e4bcec34e5 print `left 10px center` as `10px` and keep `center bottom 10px` as written under `--minify`, where it printed `50% bottom 10px`. --- src/css/values/position.rs | 35 ++++- .../css/wpt/background-computed.test.ts | 14 ++ test/js/bun/css/css.test.ts | 125 +++++++++++++++++- 3 files changed, 170 insertions(+), 4 deletions(-) diff --git a/src/css/values/position.rs b/src/css/values/position.rs index 11f87b72cd89..2ec33ad41794 100644 --- a/src/css/values/position.rs +++ b/src/css/values/position.rs @@ -149,21 +149,24 @@ impl Position { pub(crate) fn to_css(&self, dest: &mut css::Printer) -> Result<(), css::PrintErr> { if let (PositionComponent::Side(xs), PositionComponent::Length(yl)) = (&self.x, &self.y) { - if xs.side != HorizontalPositionKeyword::Left { + if xs.offset.is_some() && xs.side != HorizontalPositionKeyword::Left { self.x.to_css(dest)?; dest.write_str(" top ")?; return yl.to_css(dest); } } if let PositionComponent::Side(xs) = &self.x { - if xs.side != HorizontalPositionKeyword::Left && self.y.is_center() { + if xs.offset.is_some() + && xs.side != HorizontalPositionKeyword::Left + && self.y.is_center() + { // If there is a side keyword with an offset, "center" must be a keyword not a percentage. self.x.to_css(dest)?; return dest.write_str(" center"); } } if let (PositionComponent::Length(xl), PositionComponent::Side(ys)) = (&self.x, &self.y) { - if ys.side != VerticalPositionKeyword::Top { + if ys.offset.is_some() && ys.side != VerticalPositionKeyword::Top { dest.write_str("left ")?; xl.to_css(dest)?; dest.write_str(" ")?; @@ -180,6 +183,16 @@ impl Position { return xl.to_css(dest); } } + if let PositionComponent::Side(PositionComponentSide { + side: HorizontalPositionKeyword::Left, + offset: Some(xl), + }) = &self.x + { + if self.y.is_center() { + // `left 10px center` => `10px` + return xl.to_css(dest); + } + } if let PositionComponent::Side(xs) = &self.x { if xs.offset.is_none() && self.y.is_center() { let p: LengthPercentage = xs.side.into_length_percentage(); @@ -191,6 +204,18 @@ impl Position { return self.y.to_css(dest); } } + if let ( + PositionComponent::Center, + PositionComponent::Side(PositionComponentSide { + side: VerticalPositionKeyword::Bottom, + offset: Some(_), + }), + ) = (&self.x, &self.y) + { + // `center bottom 10px` must keep the keyword: `50% bottom 10px` is not valid. + dest.write_str("center ")?; + return self.y.to_css(dest); + } if let (PositionComponent::Side(xs), PositionComponent::Side(ys)) = (&self.x, &self.y) { if xs.offset.is_none() && ys.offset.is_none() { let x: LengthPercentage = xs.side.into_length_percentage(); @@ -221,6 +246,8 @@ impl Position { PositionComponent::Length(len) => { if len.is_zero() { break 'x_len Some(&zero); + } else { + break 'x_len Some(len); } } PositionComponent::Center => break 'x_len Some(&fifty), @@ -246,6 +273,8 @@ impl Position { PositionComponent::Length(len) => { if len.is_zero() { break 'y_len Some(&zero); + } else { + break 'y_len Some(len); } } PositionComponent::Center => break 'y_len Some(&fifty), diff --git a/test/bundler/css/wpt/background-computed.test.ts b/test/bundler/css/wpt/background-computed.test.ts index 21fc84cc3910..299e50cf06cd 100644 --- a/test/bundler/css/wpt/background-computed.test.ts +++ b/test/bundler/css/wpt/background-computed.test.ts @@ -56,6 +56,20 @@ describe("background-computed", () => { "12px 13px, 50% 6px, 30px -10px, -7px 8px", "12px 13px, 50% 6px, 30px -10px, -7px 8px", ); + runTest("background-position", "right 40%", "right 40%"); + runTest("background-position", "10px bottom", "10px bottom"); + // A gradient position has no three-value form: `at right top 40%` and `at left 10px bottom` are invalid. + runTest( + "background-image", + "radial-gradient(circle at right 40%, red, blue)", + "radial-gradient(circle at right 40%, red, #00f)", + ); + runTest( + "background-image", + "radial-gradient(circle at 10px bottom, red, blue)", + "radial-gradient(circle at 10px bottom, red, #00f)", + ); + runTest("background-image", "conic-gradient(at right 40%, red, blue)", "conic-gradient(at right 40%, red, #00f)"); runTest("background-position-x", "0.5em", ".5em"); runTest("background-position-x", "-20%, 10px", "-20%, 10px"); runTest("background-position-x", "center, left, right", "center, left, right"); diff --git a/test/js/bun/css/css.test.ts b/test/js/bun/css/css.test.ts index 8c308623f3c0..f42654fd9169 100644 --- a/test/js/bun/css/css.test.ts +++ b/test/js/bun/css/css.test.ts @@ -2958,8 +2958,11 @@ describe("css tests", () => { ); minify_test(".foo { background-position: bottom left }", ".foo{background-position:0 100%}"); - minify_test(".foo { background-position: left 10px center }", ".foo{background-position:10px 50%}"); + minify_test(".foo { background-position: left 10px center }", ".foo{background-position:10px}"); minify_test(".foo { background-position: right 10px center }", ".foo{background-position:right 10px center}"); + minify_test(".foo { background-position: center top 10px }", ".foo{background-position:50% 10px}"); + minify_test(".foo { background-position: center bottom 10px }", ".foo{background-position:center bottom 10px}"); + minify_test(".foo { background-position: center 10px }", ".foo{background-position:50% 10px}"); minify_test(".foo { background-position: right 10px top 20px }", ".foo{background-position:right 10px top 20px}"); minify_test(".foo { background-position: left 10px top 20px }", ".foo{background-position:10px 20px}"); minify_test( @@ -2968,6 +2971,11 @@ describe("css tests", () => { ); minify_test(".foo { background-position: left 10px top }", ".foo{background-position:10px 0}"); minify_test(".foo { background-position: bottom right }", ".foo{background-position:100% 100%}"); + minify_test(".foo { background-position: center top }", ".foo{background-position:top}"); + minify_test(".foo { background-position: center bottom }", ".foo{background-position:bottom}"); + minify_test(".foo { background-position: left center }", ".foo{background-position:0}"); + minify_test(".foo { background-position: right center }", ".foo{background-position:100%}"); + minify_test(".foo { background-position: 20px center }", ".foo{background-position:20px}"); minify_test( ".foo { background: url('img-sprite.png') no-repeat bottom right }", @@ -3420,6 +3428,121 @@ describe("css tests", () => { ); }); + // One function prints the position of background-position, of the background and mask + // shorthands, and of radial and conic gradients. The expected text is what lightningcss 1.33.0 prints. + describe("position", () => { + // [source, printed, printed with minify] + const sites = [ + ["background-position: ", "background-position: ", "background-position:"], + [ + "background: url(img.png) no-repeat", + 'background: url("img.png") no-repeat', + "background:url(img.png) no-repeat", + ], + [ + "background: radial-gradient(circle at , red, blue)", + "background: radial-gradient(circle at , red, #00f)", + "background:radial-gradient(circle at ,red,#00f)", + ], + [ + "background: repeating-radial-gradient(circle at , red, blue)", + "background: repeating-radial-gradient(circle at , red, #00f)", + "background:repeating-radial-gradient(circle at ,red,#00f)", + ], + [ + "background: conic-gradient(from 45deg at , red, blue)", + "background: conic-gradient(from 45deg at , red, #00f)", + "background:conic-gradient(from 45deg at ,red,#00f)", + ], + ["mask: url(img.png) ", 'mask: url("img.png") ', "mask:url(img.png) "], + [ + "-webkit-mask: url(img.png) ", + '-webkit-mask: url("img.png") ', + "-webkit-mask:url(img.png) ", + ], + ]; + // [as written, as printed]. Only background-position has a three-value form, so a gradient + // with `at right top 40%` or `at left 10px bottom` is invalid and a browser drops it. + const positions = [ + ["right 40%", "right 40%"], + ["10px bottom", "10px bottom"], + ["right center", "100%"], + ["50% bottom", "bottom"], + ]; + for (const [source, printed, minified] of sites) { + for (const [written, position] of positions) { + const input = `.foo { ${source.replace("", written)} }`; + cssTest(input, `.foo { ${printed.replace("", position)}; }`); + minify_test(input, `.foo{${minified.replace("", position)}}`); + } + } + + // background-position-x and background-position-y each take any axis shape. They merge into + // background-position, or into the background shorthand when the other longhands are known. + // Every cell is a value that background-position accepts. + const xs = ["center", "10px", "left", "left 10px", "right", "right 10px"]; + const ys = ["center", "20px", "top", "top 20px", "bottom", "bottom 20px"]; + // prettier-ignore + const merged = [ + // y: center 20px top top 20px bottom bottom 20px + ["center", "50% 20px", "top", "50% 20px", "bottom", "center bottom 20px"], // x: center + ["10px", "10px 20px", "10px 0", "10px 20px", "10px bottom", "left 10px bottom 20px"], // x: 10px + ["0", "0 20px", "0 0", "0 20px", "0 100%", "left bottom 20px"], // x: left + ["10px", "10px 20px", "10px 0", "10px 20px", "left 10px bottom", "left 10px bottom 20px"], // x: left 10px + ["100%", "right 20px", "100% 0", "right top 20px", "100% 100%", "right bottom 20px"], // x: right + ["right 10px center", "right 10px top 20px", "right 10px top", "right 10px top 20px", "right 10px bottom", "right 10px bottom 20px"], // x: right 10px + ]; + xs.forEach((x, i) => { + ys.forEach((y, j) => { + const position = merged[i][j]; + // `center center` is the only cell that minify changes. + const minified = position === "center" ? "50%" : position; + const axes = `background-position-x: ${x}; background-position-y: ${y}`; + cssTest(`.foo { ${axes} }`, `.foo { background-position: ${position}; }`); + minify_test(`.foo { ${axes} }`, `.foo{background-position:${minified}}`); + cssTest(`.foo { background: url(img.png); ${axes} }`, `.foo { background: url("img.png") ${position}; }`); + minify_test(`.foo { background: url(img.png); ${axes} }`, `.foo{background:url(img.png) ${minified}}`); + }); + }); + + // An axis longhand after background-position replaces that axis. + cssTest( + ".foo { background-position: left 10px top 5px; background-position-y: 40% }", + ".foo { background-position: 10px 40%; }", + ); + minify_test( + ".foo { background-position: left 10px top 5px; background-position-y: 40% }", + ".foo{background-position:10px 40%}", + ); + // The axes merge across adjacent rules with the same selector. + cssTest( + ".foo { background-position-x: 10px } .foo { background-position-y: top 5px }", + ".foo { background-position: 10px 5px; }", + ); + minify_test( + ".foo { background-position-x: 10px } .foo { background-position-y: top 5px }", + ".foo{background-position:10px 5px}", + ); + // background-position after the axis longhands replaces both axes. + cssTest( + ".foo { background-position-x: right 10px; background-position-y: 20px; background-position: 10px bottom }", + ".foo { background-position: 10px bottom; }", + ); + minify_test( + ".foo { background-position-x: right 10px; background-position-y: 20px; background-position: 10px bottom }", + ".foo{background-position:10px bottom}", + ); + // Each layer merges on its own. + cssTest( + ".foo { background-position-x: 10px, right, center; background-position-y: top 5px, 40%, bottom 10px }", + ".foo { background-position: 10px 5px, right 40%, center bottom 10px; }", + ); + minify_test( + ".foo { background-position-x: 10px, right, center; background-position-y: top 5px, 40%, bottom 10px }", + ".foo{background-position:10px 5px,right 40%,center bottom 10px}", + ); + }); + describe("flex", () => { cssTest( `