[css-conditional] CSS.supports with one string should accept paren-less declaration.

[css-conditional] CSS.supports with one string should accept paren-less declaration.

Relevant spec: https://drafts.csswg.org/css-conditional/#the-css-interface

> When invoked with a single conditionText argument, it must return true if
> conditionText, when either parsed and evaluated as a supports_condition or
> parsed as a declaration, wrapped in implied parentheses, and evaluated as a
> supports_condition, would return true. Otherwise, it must return false.

(Note the "wrapped in implied parentheses").

BUG=729403
Review-Url: https://codereview.chromium.org/2919283002
Cr-Commit-Position: refs/heads/master@{#477264}
Committed: https://chromium.googlesource.com/chromium/src/+/b5416b84aba8ec3442788d6bbea54091ef18b6b0

[emilio, 2017-06-04 15:00:52]

The CQ bit was checked by ecobos@igalia.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-04 15:00:56]

Dry run: CQ is trying da patch.

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

[emilio, 2017-06-04 15:02:35]

ecobos@igalia.com changed reviewers:
+ meade@chromium.org

[emilio, 2017-06-04 15:02:35]

Hi Eddy, could you take a look at this patch?

Thanks a lot in advance!

[commit-bot: I haz the power, 2017-06-04 16:46:07]

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

[commit-bot: I haz the power, 2017-06-04 16:46:09]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[emilio, 2017-06-04 19:55:03]

On 2017/06/04 15:02:35, emilio wrote:
> Hi Eddy, could you take a look at this patch?
> 
> Thanks a lot in advance!

Hmm, this is wrong in presence of compound stuff like CSS.supports("(foo) or
(bar) or (baz)") (which should parse but doesn't now. I'll fix and add a
test-case.

[emilio, 2017-06-05 00:17:26]

On 2017/06/04 19:55:03, emilio wrote:
> On 2017/06/04 15:02:35, emilio wrote:
> > Hi Eddy, could you take a look at this patch?
> > 
> > Thanks a lot in advance!
> 
> Hmm, this is wrong in presence of compound stuff like CSS.supports("(foo) or
> (bar) or (baz)") (which should parse but doesn't now. I'll fix and add a
> test-case.

This was of course wrong, and I've added a comment on why, and also cleaned up
and added a bunch of tests, so this should be ready for review.

[emilio, 2017-06-05 00:17:47]

The CQ bit was checked by ecobos@igalia.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-05 00:17:51]

Dry run: 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, 2017-06-05 02:27:42]

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

[commit-bot: I haz the power, 2017-06-05 02:27:43]

Dry run: This issue passed the CQ dry run.

[meade_UTC10, 2017-06-06 05:22:05]

I think that looks fine. lgtm

https://codereview.chromium.org/2919283002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSSupportsParser.cpp (right):

https://codereview.chromium.org/2919283002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSSupportsParser.cpp:16: //
CSS.supports, but major browser vendors allow it in CSS.supports also.
I noticed that you added a test in supports-dom-api.html:

shouldBeTrue('CSS.supports("  display: none ")');

Does that mean this comment is out of date?

[emilio, 2017-06-06 09:27:19]

On 2017/06/06 05:22:05, meade_UTC10 wrote:
> I think that looks fine. lgtm
> 
>
https://codereview.chromium.org/2919283002/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/css/parser/CSSSupportsParser.cpp (right):
> 
>
https://codereview.chromium.org/2919283002/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/css/parser/CSSSupportsParser.cpp:16: //
> CSS.supports, but major browser vendors allow it in CSS.supports also.
> I noticed that you added a test in supports-dom-api.html:
> 
> shouldBeTrue('CSS.supports("  display: none ")');
> 
> Does that mean this comment is out of date?

Yes, I think so, good catch! The spec no longer mentions any restrictions wrt
whitespace, and the wrapping in parenthesis should mean it's allowed, I removed
it.

[emilio, 2017-06-06 09:27:33]

The CQ bit was checked by ecobos@igalia.com

[emilio, 2017-06-06 09:27:33]

The patchset sent to the CQ was uploaded after l-g-t-m from meade@chromium.org
Link to the patchset: https://codereview.chromium.org/2919283002/#ps60001
(title: "Remove outdated comment.")

[commit-bot: I haz the power, 2017-06-06 09:27:46]

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, 2017-06-06 12:03:09]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1496741253077530,
"parent_rev": "1fb6007f25ce38b50bd6700f292e65ca8e501e7b", "commit_rev":
"b5416b84aba8ec3442788d6bbea54091ef18b6b0"}

[commit-bot: I haz the power, 2017-06-06 12:03:20]

Description was changed from

==========
[css-conditional] CSS.supports with one string should accept paren-less
declaration.

Relevant spec: https://drafts.csswg.org/css-conditional/#the-css-interface

> When invoked with a single conditionText argument, it must return true if
> conditionText, when either parsed and evaluated as a supports_condition or
> parsed as a declaration, wrapped in implied parentheses, and evaluated as a
> supports_condition, would return true. Otherwise, it must return false.

(Note the "wrapped in implied parentheses").

BUG=729403
==========

to

==========
[css-conditional] CSS.supports with one string should accept paren-less
declaration.

Relevant spec: https://drafts.csswg.org/css-conditional/#the-css-interface

> When invoked with a single conditionText argument, it must return true if
> conditionText, when either parsed and evaluated as a supports_condition or
> parsed as a declaration, wrapped in implied parentheses, and evaluated as a
> supports_condition, would return true. Otherwise, it must return false.

(Note the "wrapped in implied parentheses").

BUG=729403

Review-Url: https://codereview.chromium.org/2919283002
Cr-Commit-Position: refs/heads/master@{#477264}
Committed:
https://chromium.googlesource.com/chromium/src/+/b5416b84aba8ec3442788d6bbea5...
==========

[commit-bot: I haz the power, 2017-06-06 12:03:22]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/b5416b84aba8ec3442788d6bbea5...