Skip to content
New issue

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

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

Already on GitHub? # to your account

Apple Pay styling should not be treated as color #217

Closed
RReverser opened this issue Aug 14, 2018 · 5 comments
Closed

Apple Pay styling should not be treated as color #217

RReverser opened this issue Aug 14, 2018 · 5 comments

Comments

@RReverser
Copy link
Contributor

Safari has special vendored CSS properties for Apple Pay button styling: https://developer.apple.com/documentation/apple_pay_on_the_web/displaying_apple_pay_buttons

As seen from the docs, one of these properties might look like following:

    .apple-pay-button-black {
        -apple-pay-button-style: black;
    }
    .apple-pay-button-white {
        -apple-pay-button-style: white;
    }
    .apple-pay-button-white-with-line {
        -apple-pay-button-style: white-outline;
    }

Currently minifier assumes that black and white are regular CSS colors, no matter what property they're in, and minifies them to #000 and #fff correspondingly, which break styling, as for Safari these are expected to be regular names.

This could be treated as a browser-specific quirks and disabled just for that particular property, but blacklisting doesn't like a very failproof solution, since there are other contexts where regular identifiers might potentially have name collisions with colors, but shouldn't be treated as such.

@tdewolff
Copy link
Owner

Maybe we should blacklist browser specific statements in general? This would also disable minification for the range of webkit similar statements though..

@RReverser
Copy link
Contributor Author

I think it's probably better to assume that something is a colour only when it's used in a position or at least a property that definitely can hold colours, and treat it as a generic identifier otherwise. This might potentially degrade some minification, but would be more fail- and future-proof and would always allow to extend that whitelist if required.

@tdewolff
Copy link
Owner

tdewolff commented Dec 3, 2019

Hey @RReverser, I know it's been a long time ago, but if you have time it would be great if you could test the latest push to master before I publish a new version. The CSS minifier has had some refactoring and will pave the path for more minifications in the future (see #234).

@RReverser
Copy link
Contributor Author

@tdewolff Hey! Sorry, I'm actually not working on the same project and even company anymore, but cc-ing @sejoker who, I believe, has taken over the project.

@sejoker
Copy link

sejoker commented Dec 9, 2019

@tdewolff currently we don't have an automated process to test latest changes in minify versions but we in the process of building it thanks to @xtuc. I hope we will be in the better position to provide feedback on the recent changes to the library, thanks for doing great work!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants