Implement ::placeholder CSS selector.

Implement ::placeholder CSS selector.

This CL adds ::placeholder pseudo element selector which is independent from
::-webkit-input-placeholder.  This CL won't change any behavior of
::-webkit-input-placeholder, and an internal placeholder element matches to both of
selectors.

BUG=623345
Committed: https://crrev.com/573cef7f96c99540bea732d3425f6fbc8938174f
Cr-Commit-Position: refs/heads/master@{#433585}

[tkent, 2016-11-16 07:51:18]

Description was changed from

==========
Implement ::placeholder CSS selector.

work-in-progress

BUG=623345
==========

to

==========
Implement ::placeholder CSS selector.

work-in-progress.
TODO(tkent): Add tests.

BUG=623345
==========

[tkent, 2016-11-16 07:55:25]

tkent@chromium.org changed reviewers:
+ kochi@chromium.org, rune@opera.com

[tkent, 2016-11-16 07:55:27]

rune@, kochi@, what do you think about this approach?

I made a patch to introduce PseudoType::PseudoPlaceholder too, but it was more
complicated than this approach.

[rune, 2016-11-16 09:12:15]

On 2016/11/16 07:55:27, tkent wrote:
> rune@, kochi@, what do you think about this approach?
> 
> I made a patch to introduce PseudoType::PseudoPlaceholder too, but it was more
> complicated than this approach.

I think we should map the -webkit prefixed directly to the unprefixed one during
parsing so that the serialization also does the unprefixed one. At least, we
serialize the non-prefixed version for aliases like -webkit-animation.

I've asked simonp@opera.com about what an alias means in terms of cssom. It's
not defined in https://compat.spec.whatwg.org and he asked me to file an issue.

[tkent, 2016-11-17 05:56:24]

On 2016/11/16 at 09:12:15, rune wrote:
> On 2016/11/16 07:55:27, tkent wrote:
> > rune@, kochi@, what do you think about this approach?
> > 
> > I made a patch to introduce PseudoType::PseudoPlaceholder too, but it was
more
> > complicated than this approach.
> 
> I think we should map the -webkit prefixed directly to the unprefixed one
during parsing so that the serialization also does the unprefixed one. At least,
we serialize the non-prefixed version for aliases like -webkit-animation.

Thank you for the suggestion. It will break the code like [1], right?  I think
changing -webkit-input-placeholder behavior is high comptibility-risk.

[1]
http://stackoverflow.com/questions/28592895/trying-to-get-style-of-placeholde...

[rune, 2016-11-17 10:08:23]

On 2016/11/17 05:56:24, tkent wrote:
> On 2016/11/16 at 09:12:15, rune wrote:
> > On 2016/11/16 07:55:27, tkent wrote:
> > > rune@, kochi@, what do you think about this approach?
> > > 
> > > I made a patch to introduce PseudoType::PseudoPlaceholder too, but it was
> more
> > > complicated than this approach.
> > 
> > I think we should map the -webkit prefixed directly to the unprefixed one
> during parsing so that the serialization also does the unprefixed one. At
least,
> we serialize the non-prefixed version for aliases like -webkit-animation.
> 
> Thank you for the suggestion. It will break the code like [1], right?  I think
> changing -webkit-input-placeholder behavior is high comptibility-risk.
> 
> [1]
>
http://stackoverflow.com/questions/28592895/trying-to-get-style-of-placeholde...

[rune, 2016-11-17 10:25:08]

On 2016/11/17 05:56:24, tkent wrote:

> Thank you for the suggestion. It will break the code like [1], right?  I think
> changing -webkit-input-placeholder behavior is high comptibility-risk.

I've replied to the intent thread for others to chime in.

[rune, 2016-11-18 09:53:23]

https://codereview.chromium.org/2509593002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/CSSSelector.cpp (right):

https://codereview.chromium.org/2509593002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/CSSSelector.cpp:360: {"placeholder",
CSSSelector::PseudoWebKitCustomElement},
Making this a PseudoWebKitCustomElement means this pseudo element will get the
same treatment as -webkit-prefixed pseudo elements in certain parts of the code
like isPseudoClassValidAfterPseudoElement(). For the mentioned method it means
we're allowing user action pseudo classes after the element. What would the code
look like if we actually assigned a separate pseudo element enum for this?

https://codereview.chromium.org/2509593002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/CSSSelector.cpp:441: AtomicString
CSSSelector::aliasForShadowPseudoId(const AtomicString& name) {
Using the term alias here could be confusing since you're not implementing it as
an alias in terms of the whatwg compat spec, but just sharing code between the
implementations.

Not sure what a better name would be.

[tkent, 2016-11-21 03:55:54]

The CQ bit was checked by tkent@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 03:56:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tkent, 2016-11-21 04:00:02]

The CQ bit was checked by tkent@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 04:00:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tkent, 2016-11-21 04:42:17]

Description was changed from

==========
Implement ::placeholder CSS selector.

work-in-progress.
TODO(tkent): Add tests.

BUG=623345
==========

to

==========
Implement ::placeholder CSS selector.

This CL adds ::placeholder pseudo element selector which is independent from
::-webkit-input-placeholder.  This CL won't change any behavior of
::-webkit-input-placeholder, and an internal placeholder element matches to both
of
selectors.

BUG=623345
==========

[tkent, 2016-11-21 05:36:02]

https://codereview.chromium.org/2509593002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/CSSSelector.cpp (right):

https://codereview.chromium.org/2509593002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/CSSSelector.cpp:360: {"placeholder",
CSSSelector::PseudoWebKitCustomElement},
On 2016/11/18 at 09:53:23, rune wrote:
> Making this a PseudoWebKitCustomElement means this pseudo element will get the
same treatment as -webkit-prefixed pseudo elements in certain parts of the code
like isPseudoClassValidAfterPseudoElement(). For the mentioned method it means
we're allowing user action pseudo classes after the element. What would the code
look like if we actually assigned a separate pseudo element enum for this?

Firefox and Safari don't accept user-action pseudo classes after ::placeholder. 
So this approach wasn't good.

Patch Set 2 introduces new separated enum.  Would you take another look at it
please?
I also added tests.

[commit-bot: I haz the power, 2016-11-21 07:55:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 07:55:49]

Dry run: This issue passed the CQ dry run.

[rune, 2016-11-21 08:52:33]

https://codereview.chromium.org/2509593002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/2509593002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/TestExpectations:1335: crbug.com/623345
fast/forms/textarea/textarea-placeholder-pseudo-style.html [ NeedsRebaseline ]
Would it be a lot of work to make this a ref-test now?

https://codereview.chromium.org/2509593002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/css/pseudo-placeholder.html (right):

https://codereview.chromium.org/2509593002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css/pseudo-placeholder.html:17: // There are
three rules.  But the first one should not be counted.
"should not be counted" -> "should be dropped"

https://codereview.chromium.org/2509593002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/ElementRuleCollector.cpp (right):

https://codereview.chromium.org/2509593002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/ElementRuleCollector.cpp:200: cascadeOrder,
matchRequest);
This means we'll try to match ::placeholder rules for all custom pseudo
elements, right? Should be guarded by if-test for placeholder pseudo id?

[tkent, 2016-11-21 09:23:39]

https://codereview.chromium.org/2509593002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/2509593002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/TestExpectations:1335: crbug.com/623345
fast/forms/textarea/textarea-placeholder-pseudo-style.html [ NeedsRebaseline ]
On 2016/11/21 at 08:52:33, rune wrote:
> Would it be a lot of work to make this a ref-test now?

Not much.  Done.

https://codereview.chromium.org/2509593002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/css/pseudo-placeholder.html (right):

https://codereview.chromium.org/2509593002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css/pseudo-placeholder.html:17: // There are
three rules.  But the first one should not be counted.
On 2016/11/21 at 08:52:33, rune wrote:
> "should not be counted" -> "should be dropped"

Done.

https://codereview.chromium.org/2509593002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/ElementRuleCollector.cpp (right):

https://codereview.chromium.org/2509593002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/ElementRuleCollector.cpp:200: cascadeOrder,
matchRequest);
On 2016/11/21 at 08:52:33, rune wrote:
> This means we'll try to match ::placeholder rules for all custom pseudo
elements, right? Should be guarded by if-test for placeholder pseudo id?

Oh, that's right.  Fixed.

[rune, 2016-11-21 09:42:53]

lgtm with nits.

PS. I don't understand why the Mac-expectations are M and not D.

https://codereview.chromium.org/2509593002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/2509593002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/TestExpectations:1335: crbug.com/623345
fast/forms/textarea/textarea-placeholder-pseudo-style.html [ NeedsRebaseline ]
On 2016/11/21 09:23:39, tkent wrote:
> On 2016/11/21 at 08:52:33, rune wrote:
> > Would it be a lot of work to make this a ref-test now?
> 
> Not much.  Done.

Great!

https://codereview.chromium.org/2509593002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/forms/text/placeholder-pseudo-style.html
(right):

https://codereview.chromium.org/2509593002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/forms/text/placeholder-pseudo-style.html:1:
<style>
<!DOCTYPE html>?

https://codereview.chromium.org/2509593002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/forms/textarea/textarea-placeholder-pseudo-style.html
(right):

https://codereview.chromium.org/2509593002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/forms/textarea/textarea-placeholder-pseudo-style.html:1:
<style>
Unless this is intentionally relying on quirks mode, you should add a <!DOCTYPE
html>. Same for *-expected.html

[tkent, 2016-11-21 13:14:12]

The CQ bit was checked by tkent@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 13:14:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tkent, 2016-11-21 14:42:07]

https://codereview.chromium.org/2509593002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/forms/text/placeholder-pseudo-style.html
(right):

https://codereview.chromium.org/2509593002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/forms/text/placeholder-pseudo-style.html:1:
<style>
On 2016/11/21 at 09:42:53, rune wrote:
> <!DOCTYPE html>?

Done.

https://codereview.chromium.org/2509593002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/forms/textarea/textarea-placeholder-pseudo-style.html
(right):

https://codereview.chromium.org/2509593002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/forms/textarea/textarea-placeholder-pseudo-style.html:1:
<style>
On 2016/11/21 at 09:42:53, rune wrote:
> Unless this is intentionally relying on quirks mode, you should add a
<!DOCTYPE html>. Same for *-expected.html

Done.

[tkent, 2016-11-21 14:43:26]

On 2016/11/21 at 09:42:53, rune wrote:
> PS. I don't understand why the Mac-expectations are M and not D.

It's a known issue of our code review tool.  If a file in an earlier patch set
is M, it won't be D in later patch set.

[tkent, 2016-11-21 14:44:19]

The CQ bit was unchecked by tkent@chromium.org

[tkent, 2016-11-21 14:44:22]

The CQ bit was checked by tkent@chromium.org

[tkent, 2016-11-21 14:44:22]

The patchset sent to the CQ was uploaded after l-g-t-m from rune@opera.com
Link to the patchset: https://codereview.chromium.org/2509593002/#ps60001
(title: "<!DOCTYPE html>")

[commit-bot: I haz the power, 2016-11-21 14:44:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-21 15:21:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 15:21:28]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[tkent, 2016-11-21 16:12:00]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2016-11-21 16:12:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-21 18:05:24]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1479744720999800,
"parent_rev": "5431ac40a7f48c62cbc455e6eae027b3d2e5d958", "commit_rev":
"b006c6fa29b1789b9dfe818895d31a8453eb002a"}

[commit-bot: I haz the power, 2016-11-21 18:05:51]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-21 18:09:15]

Description was changed from

==========
Implement ::placeholder CSS selector.

This CL adds ::placeholder pseudo element selector which is independent from
::-webkit-input-placeholder.  This CL won't change any behavior of
::-webkit-input-placeholder, and an internal placeholder element matches to both
of
selectors.

BUG=623345
==========

to

==========
Implement ::placeholder CSS selector.

This CL adds ::placeholder pseudo element selector which is independent from
::-webkit-input-placeholder.  This CL won't change any behavior of
::-webkit-input-placeholder, and an internal placeholder element matches to both
of
selectors.

BUG=623345

Committed: https://crrev.com/573cef7f96c99540bea732d3425f6fbc8938174f
Cr-Commit-Position: refs/heads/master@{#433585}
==========

[commit-bot: I haz the power, 2016-11-21 18:09:16]

Patchset 4 (id:??) landed as
https://crrev.com/573cef7f96c99540bea732d3425f6fbc8938174f
Cr-Commit-Position: refs/heads/master@{#433585}