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

Removed vendor prefixes, except where it fixes a specific bug or quirk #561

Merged
merged 1 commit into from
Nov 6, 2014

Conversation

mikemike
Copy link
Contributor

In reference to issue #558 (#558).

Some vendor prefixes remain, but only those that explicitly fix a bug or quirk on a device (there are 2 or 3 that fix bugs with the S5).

@obenland
Copy link
Member

Why do we remove -webkit-box-sizing on *, *:before, *:after but not on input[type="search"]?

@mikemike
Copy link
Contributor Author

The rule on input[type=search] specified that it fixed a quirk with S5 and Chrome, so it was left. I assumed this was a quirk specific to input[type=search]. S5 is a relatively new device, so I wouldn't want to remove it.

@philiparthurmoore
Copy link
Collaborator

It's hard to read through your changes and know why you deleted the lines that you deleted. It would help if you could add supporting information into this Pull Request thread or directly on the lines that changed so that it's easier to understand why you took them out.

@mikemike
Copy link
Contributor Author

@philiparthurmoore, the lines were removed due to them only being there to support browsers with very low market share (which may not have had such a low share at time of original authoring). This was discussed in issue #558 to lower the number of generated warnings. That's an overall view of the changes, but I will annotate each individually shortly.

@@ -77,8 +77,6 @@ html {
*,
*:before,
*:after { /* apply a natural box layout model to all elements; see http://www.paulirish.com/2012/box-sizing-border-box-ftw/ */
-webkit-box-sizing: border-box; /* Not needed for modern webkit but still used by Blackberry Browser 7.0; see http://caniuse.com/#search=box-sizing */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed due to low market share, less than 0.1%.

@philiparthurmoore
Copy link
Collaborator

@mikemike Thank you very much. Sometimes it's hard to keep track of everything that's going on so the notes really help out. Cheers.

@@ -358,7 +356,6 @@ input[type="radio"] {
input[type="search"] {
-webkit-appearance: textfield; /* Addresses appearance set to searchfield in S5, Chrome */
-webkit-box-sizing: content-box; /* Addresses box sizing set to border-box in S5, Chrome (include -moz to future-proof) */
-moz-box-sizing: content-box;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Webkit vendor prefixes were kept here due to them fixing a bug/quirk in a relatively new browser whose market share is likely growing rather than shrinking. box-sizing is supported in Firefox without prefix since version 29.

@mikemike
Copy link
Contributor Author

@philiparthurmoore no, totally agree - much easier to read annotated code than an issue thread.

Let me know if you disagree with any of the changes made. I have used best judgement, so people may wish to keep some of these, or remove others.

@ghost
Copy link

ghost commented Aug 12, 2014

my suggestion is to include WordPress' autoprefixer suggestions:

autoprefixer: {
    options: {
        browsers: ['Android >= 2.1', 'Chrome >= 21', 'Explorer >= 7', 'Firefox >= 17', 'Opera >= 12.1', 'Safari >= 6.0']
    },

@mindctrl
Copy link

'Explorer >= 7'

Wasn't IE 6-8 support dropped?

@obenland obenland merged commit 16587b5 into Automattic:master Nov 6, 2014
obenland added a commit that referenced this pull request Nov 6, 2014
Also removes the box-sizing mixin, as it is of no use anymore.

See http://caniuse.com/#feat=css3-boxsizing
See #558.

Fixes #561.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants