Ensure that we don't allow 'none' alongside other values in content property

Ensure that we don't allow 'none' alongside other values in content property

Currently Chrome accepts a list of values that include "none" along
with other values as an acceptable input for CSS property content. This
breaks from the spec stated here:
https://drafts.csswg.org/css2/generate.html#content
where "none" is only accepted as a valid input if it is the
only value assigned to content. This patch fixes that behaviour.
FF, IE and Edge are consistent with spec here.

Committed: https://crrev.com/8845c5f6e9b2e7e84642cf716fc99b4cbfd2a53d
Cr-Commit-Position: refs/heads/master@{#383946}

[nainar, 2016-03-30 01:46:01]

nainar@chromium.org changed reviewers:
+ timloh@chromium.org

[nainar, 2016-03-30 01:55:07]

Description was changed from

==========
Accept "none" as a single ident value in content property

Currently Chrome doesn't accept none as a single ident value for the
content property. This breaks from the spec stated here:
https://drafts.csswg.org/css2/generate.html#content
which states that content can be set to "none" as a single value.
This patch fixes that behaviour.
Other browsers (FF, IE and Edge) are consistent with spec here.
==========

to

==========
Accept "none" as a single ident value in content property

Currently Chrome doesn't accept none as a single ident value for the
content property. This breaks from the spec stated here:
https://drafts.csswg.org/css2/generate.html#content
which states that content can be set to "none" as a single value.
This patch fixes that behaviour.
==========

[nainar, 2016-03-30 02:33:28]

Description was changed from

==========
Accept "none" as a single ident value in content property

Currently Chrome doesn't accept none as a single ident value for the
content property. This breaks from the spec stated here:
https://drafts.csswg.org/css2/generate.html#content
which states that content can be set to "none" as a single value.
This patch fixes that behaviour.
==========

to

==========
Ensure that we don't allow 'none' alongside other values in content property

Currently Chrome accepts a list of values that include "none" along
with other values as an acceptable input for CSS property content. This
breaks from the spec stated here:
https://drafts.csswg.org/css2/generate.html#content
where "none" is only accepted as a valid input if it is the
only value assigned to content. This patch fixes that behaviour.
FF, IE and Edge are consistent with spec here.
==========

[Timothy Loh, 2016-03-30 02:53:29]

https://codereview.chromium.org/1837413002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/css-parser/content-parsing-invalid.html
(right):

https://codereview.chromium.org/1837413002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/css-parser/content-parsing-invalid.html:14:
content: none normal;
It'd make more sense to test something like none open-quote or url("a") none.
Prior to this patch, this test would pass anyway.

https://codereview.chromium.org/1837413002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/css-parser/content-parsing-invalid.html:26:
assert_equals(getComputedStyle(target1, '::before').content, '');
btw I was too lazy to whine about this previously but it'd be better to test
this by check document.styleSheets[0].cssRules[0].style.content and similar.
getComputedStyle requires more of the pipeline than we should depend on for a
test like this. This way you won't end up needing to change it in the other
patch...

https://codereview.chromium.org/1837413002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/css-parser/content-parsing.html (right):

https://codereview.chromium.org/1837413002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/css-parser/content-parsing.html:25:
assert_invalid_value("content", "none open-quote");
repeated test :|

https://codereview.chromium.org/1837413002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp (right):

https://codereview.chromium.org/1837413002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp:662: if
(cssValueID == CSSValueNormal || cssValueID == CSSValueNone) {
why did the assert become an if?

[nainar, 2016-03-30 03:27:36]

https://codereview.chromium.org/1837413002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/css-parser/content-parsing-invalid.html
(right):

https://codereview.chromium.org/1837413002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/css-parser/content-parsing-invalid.html:14:
content: none normal;
Done.

https://codereview.chromium.org/1837413002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/css-parser/content-parsing-invalid.html:26:
assert_equals(getComputedStyle(target1, '::before').content, '');
Done.

https://codereview.chromium.org/1837413002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/css-parser/content-parsing.html (right):

https://codereview.chromium.org/1837413002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/css-parser/content-parsing.html:25:
assert_invalid_value("content", "none open-quote");
Removed.

https://codereview.chromium.org/1837413002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp (right):

https://codereview.chromium.org/1837413002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp:662: if
(cssValueID == CSSValueNormal || cssValueID == CSSValueNone) {
Switched back.

[Timothy Loh, 2016-03-30 03:32:07]

lgtm

[nainar, 2016-03-30 03:34:15]

The CQ bit was checked by nainar@chromium.org

[commit-bot: I haz the power, 2016-03-30 03:34:49]

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

[commit-bot: I haz the power, 2016-03-30 09:25:39]

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

[commit-bot: I haz the power, 2016-03-30 09:25:40]

Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_TIMED_OUT,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_TIMED_OUT,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on tryserver.chromium.win (JOB_TIMED_OUT,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_TIMED_OUT,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[nainar, 2016-03-30 09:31:20]

The CQ bit was checked by nainar@chromium.org

[commit-bot: I haz the power, 2016-03-30 09:31:41]

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

[commit-bot: I haz the power, 2016-03-30 11:02:06]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-03-30 11:02:55]

Description was changed from

==========
Ensure that we don't allow 'none' alongside other values in content property

Currently Chrome accepts a list of values that include "none" along
with other values as an acceptable input for CSS property content. This
breaks from the spec stated here:
https://drafts.csswg.org/css2/generate.html#content
where "none" is only accepted as a valid input if it is the
only value assigned to content. This patch fixes that behaviour.
FF, IE and Edge are consistent with spec here.
==========

to

==========
Ensure that we don't allow 'none' alongside other values in content property

Currently Chrome accepts a list of values that include "none" along
with other values as an acceptable input for CSS property content. This
breaks from the spec stated here:
https://drafts.csswg.org/css2/generate.html#content
where "none" is only accepted as a valid input if it is the
only value assigned to content. This patch fixes that behaviour.
FF, IE and Edge are consistent with spec here.

Committed: https://crrev.com/8845c5f6e9b2e7e84642cf716fc99b4cbfd2a53d
Cr-Commit-Position: refs/heads/master@{#383946}
==========

[commit-bot: I haz the power, 2016-03-30 11:02:56]

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