Make <attr-name> accept any CSS identifier

Make <attr-name> accept any CSS identifier

Make <attr-name> accept any CSS identifier rather than
rejecting those starting with minus.
With this change we do match attributes starting with minus
unless they are dynamically added.

Behavior matches Firefox.

Committed: https://crrev.com/50b643c442f43bba16bdc9355e53729d6dd5c87c
Cr-Commit-Position: refs/heads/master@{#391412}

[rwlbuis, 2016-04-28 20:24:11]

Description was changed from

==========
attr() WIP

BUG=
==========

to

==========
Make <attr-name> accept any CSS identifier

Make <attr-name> accept any CSS identifier rather than rejecting those starting
with '-'.
Even though we now accept identifiers starting with '-' it
will still not match since attributes can't start with '-'.

Behavior matches Firefox.
==========

[rwlbuis, 2016-04-28 21:03:33]

Description was changed from

==========
Make <attr-name> accept any CSS identifier

Make <attr-name> accept any CSS identifier rather than rejecting those starting
with '-'.
Even though we now accept identifiers starting with '-' it
will still not match since attributes can't start with '-'.

Behavior matches Firefox.
==========

to

==========
Make <attr-name> accept any CSS identifier

Make <attr-name> accept any CSS identifier rather than
rejecting those starting with minus.
With this change we do match attributes starting with minus
unless they are dynamically added.

Behavior matches Firefox.
==========

[rwlbuis, 2016-04-28 21:06:22]

rob.buis@samsung.com changed reviewers:
+ timloh@chromium.org

[rwlbuis, 2016-04-28 21:06:22]

PTAL.

Since the HTML parser accepts attributes starting with minus (AFAIK it should
not) I guess we can't really make this change. Maybe it is better to update the
comment to indicate that we do not accept "-" because of above?
Note that Firefox does have this new behavior, but it seems inconsistent for the
web developer if implemented like this.

[rune, 2016-05-03 11:03:43]

On 2016/04/28 21:06:22, rwlbuis wrote:
> PTAL.
> 
> Since the HTML parser accepts attributes starting with minus (AFAIK it should
> not) I guess we can't really make this change. Maybe it is better to update
the
> comment to indicate that we do not accept "-" because of above?
> Note that Firefox does have this new behavior, but it seems inconsistent for
the
> web developer if implemented like this.


We're incompatible with the current behavior as well where:

  div::before { attr(-ignored) "PASS" }

generates a "PASS" for Gecko, while the whole rule is empty in Blink.

To me this is an lgtm with nits.

[rune, 2016-05-03 11:03:51]

rune@opera.com changed reviewers:
+ rune@opera.com

[rune, 2016-05-03 11:03:52]

https://codereview.chromium.org/1924313002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/css-generated-content/attr-minus.html
(right):

https://codereview.chromium.org/1924313002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css-generated-content/attr-minus.html:4: dv
{ display: none; }
What's this rule for?

https://codereview.chromium.org/1924313002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css-generated-content/attr-minus.html:7:
<div -foo="PASS"></div>
Should this test have a comment saying that the selector is valid, but the
element should not match because the attribute should have been dropped?

[rune, 2016-05-03 11:05:21]

FTR, we currently accept dash-prefixed element selectors, yet the never match as
the parser does not accept those as tag names.

[rwlbuis, 2016-05-03 14:38:52]

rune thanks for the review and thumbs up. I'll wait a bit for @timloh's feedback
before landing.

https://codereview.chromium.org/1924313002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/css-generated-content/attr-minus.html
(right):

https://codereview.chromium.org/1924313002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css-generated-content/attr-minus.html:4: dv
{ display: none; }
On 2016/05/03 11:03:52, rune wrote:
> What's this rule for?

It is a left-over from my first attempt at this test, removed.

https://codereview.chromium.org/1924313002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css-generated-content/attr-minus.html:7:
<div -foo="PASS"></div>
On 2016/05/03 11:03:52, rune wrote:
> Should this test have a comment saying that the selector is valid, but the
> element should not match because the attribute should have been dropped?

Done.

[Timothy Loh, 2016-05-04 00:51:07]

lgtm

[rwlbuis, 2016-05-04 00:57:47]

The CQ bit was checked by rob.buis@samsung.com

[rwlbuis, 2016-05-04 00:57:48]

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/1924313002/#ps60001
(title: "Address review comments")

[commit-bot: I haz the power, 2016-05-04 00:58:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1924313002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1924313002/60001

[commit-bot: I haz the power, 2016-05-04 02:13:03]

Description was changed from

==========
Make <attr-name> accept any CSS identifier

Make <attr-name> accept any CSS identifier rather than
rejecting those starting with minus.
With this change we do match attributes starting with minus
unless they are dynamically added.

Behavior matches Firefox.
==========

to

==========
Make <attr-name> accept any CSS identifier

Make <attr-name> accept any CSS identifier rather than
rejecting those starting with minus.
With this change we do match attributes starting with minus
unless they are dynamically added.

Behavior matches Firefox.
==========

[commit-bot: I haz the power, 2016-05-04 02:13:04]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-04 02:15:27]

Description was changed from

==========
Make <attr-name> accept any CSS identifier

Make <attr-name> accept any CSS identifier rather than
rejecting those starting with minus.
With this change we do match attributes starting with minus
unless they are dynamically added.

Behavior matches Firefox.
==========

to

==========
Make <attr-name> accept any CSS identifier

Make <attr-name> accept any CSS identifier rather than
rejecting those starting with minus.
With this change we do match attributes starting with minus
unless they are dynamically added.

Behavior matches Firefox.

Committed: https://crrev.com/50b643c442f43bba16bdc9355e53729d6dd5c87c
Cr-Commit-Position: refs/heads/master@{#391412}
==========

[commit-bot: I haz the power, 2016-05-04 02:15:28]

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