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

Doesn't properly generate input placeholder rules #189

Closed
necolas opened this issue Feb 5, 2014 · 23 comments
Closed

Doesn't properly generate input placeholder rules #189

necolas opened this issue Feb 5, 2014 · 23 comments

Comments

@necolas
Copy link

necolas commented Feb 5, 2014

source:

input::placeholder { color: black; }
input::-webkit-input-placeholder { line-height: 16px; /* fix chrome bug */ }

output:

input::-moz-placeholder { color: black; }
input:-ms-input-placeholder { color: black; }
input::placeholder { color: black; }
input::-webkit-input-placeholder { line-height: 16px; /* fix chrome bug */ }

expected:

input::-moz-placeholder { color: black; }
input:-ms-input-placeholder { color: black; }
input::-webkit-input-placeholder { color: black; }
input::placeholder { color: black; }
input::-webkit-input-placeholder { line-height: 16px; /* fix chrome bug */ }

I'd expect to be able to "extend" the generated webkit rule (in the example), rather than have it completely omitted by AP.

@ai
Copy link
Member

ai commented Feb 5, 2014

I understand, but you want Autoprefixer to be too smart and understand does vendor hack (like input::-webkit-input-placeholder here) append rules or override rules ;).

Current logick is simple. If you write vendor hack, Autoprefixer will not add own.

We can add more code, try to look inside rules. But I think, it will be bad for code climate and overengineering :).

But I open for PR :).

@ai ai closed this as completed Feb 5, 2014
@necolas
Copy link
Author

necolas commented Feb 5, 2014

I think that makes sense for properties, but not for pseudo-elements.

@ai
Copy link
Member

ai commented Feb 5, 2014

I understand, that you have some point, but them come another user and say, that he want different logic ;).

@necolas
Copy link
Author

necolas commented Feb 7, 2014

It's the CSS language logic.

@ai
Copy link
Member

ai commented Feb 7, 2014

I mean, that somebody may need miss some property in hack selector. How do I know, that you need color for webkit in your example?

@ai
Copy link
Member

ai commented Feb 7, 2014

Or maybe we should remove too smart logic and not detect hacks for selectors?

Main problems is when browsers well start support placeholders and will read both prefixed and unprefixed rules.

@ai ai reopened this Feb 7, 2014
@necolas
Copy link
Author

necolas commented Feb 7, 2014

How do I know, that you need color for webkit in your example?

What I'm thinking is that you don't need to; you pretend like it isn't there and generate the full set of rules.

Main problems is when browsers well start support placeholders and will read both prefixed and unprefixed rules.

How would that be different than having them apply 2 separate vendor-prefixed rules (see my desired output example)?

@ai ai added the enhancement label Feb 7, 2014
@lydell
Copy link
Contributor

lydell commented Feb 7, 2014

Could ::placeholder:not(::-webkit-input-placeholder) {} work, perhaps?

@ai
Copy link
Member

ai commented Feb 7, 2014

I will change logic for selectors only in 1.1, because it will broke current API.

@necolas
Copy link
Author

necolas commented Feb 7, 2014

OK sounds good. Might be something to get wider feedback on before a public release too. Thanks for considering it :)

@ai ai added the 1.1 label Feb 7, 2014
@necolas
Copy link
Author

necolas commented Feb 11, 2014

Yep, this is not working properly for us at Twitter. Say I have 2 files. One uses prefixed placeholders (or has them generated in place), the other uses ::placeholder to override them. If they are concatenated and then passed to AP, AP doesn't generate any new placeholder prefixes because it finds prefixes used in existing selectors, so the resulting "override" CSS is not as expected.

@necolas
Copy link
Author

necolas commented Feb 11, 2014

Or even the same file. You might ask "why", but this is what a bundle looks like when you append override rules for experiments.

Source:

/* from default.css */

input::placeholder { color: gray; }

/* from override.css */

input::placeholder { color: black; }

Output:

/* from default.css */

input::-moz-placeholder { color: gray; }
input:-ms-input-placeholder { color: gray; }
input::-webkit-input-placeholder { color: gray; }
input::placeholder { color: gray; }

/* from override.css */

input::placeholder { color: black; }

Clearly, if you expect the unprefixed ::placeholder to override due to the CSS cascade, then AP should be generating the prefixed selectors again. Or de-duping and putting the prefixed versions next to the last instance of the selector.

@ai
Copy link
Member

ai commented Feb 11, 2014

Hmmm, really I totally forget about this. Moving property logic to selectors is a bad idea :).

@ai
Copy link
Member

ai commented Feb 12, 2014

There will not be hotfix today :(. We need more smarter check to prevent adding prefixed selector again, when in some cases we call autoprefixer again.

But I can use logic from properties, where we view for prefixed selectors only on upper unprefixed one (also it will be faster).

For hostfix today you can put next placeholder in any at-rule to create new context, like:

@media all {
  input::placeholder { color: black; }
}

I will fix it on this or next week and them you can remove your hack.

@necolas
Copy link
Author

necolas commented Feb 12, 2014

We need more smarter check to prevent adding prefixed selector again, when in some cases we call autoprefixer again.

Yeah that definitely adds to the difficulty.

Thanks for the hack! That works for now.

@ai
Copy link
Member

ai commented Feb 15, 2014

Done 42a6539

  • Now Autoprefixer try to group selectors (prefixed can be directly before unprefixed) and remove outdated selector only if it in group.
  • Don’t prefix selector only if prefixed is in group, not in all CSS.
  • Prefix selector even if directy after unprefixed selector will be prefixed hack.

@ai ai closed this as completed Feb 15, 2014
@ai
Copy link
Member

ai commented Feb 15, 2014

I try to release 1.1 with this fix at Monday.

@necolas
Copy link
Author

necolas commented Feb 15, 2014

Thanks so much for this!

@ai
Copy link
Member

ai commented Feb 18, 2014

Version 1.1 with this fix was released today.

@hoIIer
Copy link

hoIIer commented Mar 20, 2017

tldr: what do I use for placeholder? ::placeholder?

@ai
Copy link
Member

ai commented Mar 20, 2017

@erichonkanen ::placeholder

@hoIIer
Copy link

hoIIer commented Mar 20, 2017

@ai thanks

@nikuideveloper
Copy link

reduce the padding in textbox. i am able to get the placeholder in firefox

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

No branches or pull requests

5 participants