Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Already prefixed backdrop-filter results in two prefixed declarations #403

Closed
mischnic opened this issue Jan 20, 2023 · 3 comments · Fixed by #850
Closed

Already prefixed backdrop-filter results in two prefixed declarations #403

mischnic opened this issue Jan 20, 2023 · 3 comments · Fixed by #850

Comments

@mischnic
Copy link
Member

body {
  -webkit-backdrop-filter: blur(20px);
  backdrop-filter: blur(20px);
}

results in

body {
  -webkit-backdrop-filter: blur(20px);
  -webkit-backdrop-filter: blur(20px);
  backdrop-filter: blur(20px);
}

Just like #237

Playground

@LeoniePhiline
Copy link
Contributor

LeoniePhiline commented Jul 26, 2023

In the above playground link, the result is now:

body {
  backdrop-filter: blur(20px);
}

That's a regression in lightningcss 1.21.0. (#537 (comment))

@LeoniePhiline
Copy link
Contributor

LeoniePhiline commented Jul 26, 2023

Duplicated (-webkit-backdrop-filter: blur(8px)) in @supports: Regression in lightningcss 1.18.0

All results produced with pnpm exec lightningcss --targets 'defaults' input.css -o output.css after pnpm i -D [email protected] / pnpm i -D [email protected].

The following works with lightningcss 1.17.1 and broke with lightningcss 1.18.0:

Source

@supports (
  (-webkit-backdrop-filter: blur(8px)) or (backdrop-filter: blur(8px))
) {
  :root {
    --some-color: #000000e6;
  }
}

Good result in lightningcss 1.17.1

@supports ((-webkit-backdrop-filter: blur(8px)) or (backdrop-filter: blur(8px))) {
  :root {
    --some-color: rgba(0, 0, 0, .9);
  }
}

Bad result in lightningcss 1.18.0+

@supports ((-webkit-backdrop-filter: blur(8px))) or ((-webkit-backdrop-filter: blur(8px)) or (backdrop-filter: blur(8px))) {
  :root {
    --some-color: rgba(0, 0, 0, .9);
  }
}

Missing tests

Added failing test: https://github.com/parcel-bundler/lightningcss/pull/552/files#diff-b1a35a68f14e696205874893c07fd24fdb88882b47c23cc0e0c80a30c7d53759R12557-R12576

Initial analysis

Prefixes are correct

Auto-generated src/prefixes.rs is correct, with 589824 indicating "Safari 9.0.0 and higher", and no upper bound:

https://github.com/parcel-bundler/lightningcss/blob/master/src/prefixes.rs#L557-L562

rules::supports::SupportsCondition is unaware of already-present prefixes in logical or:

SupportsCondition::Or(
    [
        Declaration {
            property_id: BackdropFilter(
                VendorPrefix(
                    WebKit,
                ),
            ),
            value: "blur(8px)",
        },
        Declaration {
            property_id: BackdropFilter(
                VendorPrefix(
                    WebKit | None,
                ),
            ),
            value: "blur(8px)",
        },
    ],
)

Here, the second SupportsCondition::Declaration prints the second -webkit-backdrop-filter, unaware of the first one in the other (preceding) Declaration.

Possible solutions

Either, at initalization time the SupportsCondition::Or should be reduced to a single Declaration with and without prefix:

SupportsCondition::Declaration {
    property_id: BackdropFilter(
        VendorPrefix(
            WebKit | None,
        ),
    ),
    value: "blur(8px)",
}

Otherwise, the SupportsCondition should be initialized without the prefix in the second declaration of the same property ID, such as:

SupportsCondition::Or(
    [
        Declaration {
            property_id: BackdropFilter(
                VendorPrefix(
                    WebKit,
                ),
            ),
            value: "blur(8px)",
        },
        Declaration {
            property_id: BackdropFilter(
                VendorPrefix(
                    None,
                ),
            ),
            value: "blur(8px)",
        },
    ],
)

@LeoniePhiline
Copy link
Contributor

LeoniePhiline commented Jul 26, 2023

Duplicated -webkit-backdrop-filter in transition-property: Broken in all versions since at least lightningcss 1.14.0

All results produced with pnpm exec lightningcss --targets 'defaults' input.css -o output.css after pnpm i -D [email protected], @1.15.0, @1.16.0, @1.75.0, @1.18.0, @1.19.0, @1.20.0, @1.21.0, @1.21.1, @1.21.2, @1.21.3, @1.21.4, @1.21.5.

The following does not work in any version since at least lightningcss 1.14.0 (older versions were apparently yanked):

Source

.transition {
  transition-property: -webkit-backdrop-filter, backdrop-filter;
}

Bad result in all available lightningcss versions

.transition {
  transition-property: -webkit-backdrop-filter, -webkit-backdrop-filter, backdrop-filter;
}

Added failing test: https://github.com/parcel-bundler/lightningcss/pull/551/files#diff-b1a35a68f14e696205874893c07fd24fdb88882b47c23cc0e0c80a30c7d53759R10506-R10523

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 a pull request may close this issue.

2 participants