New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 1
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment
link

Issue 921152: CSS.supports / @supports doesn't consider valid any value with custom property references / env.

Reported by emilio@chromium.org, Jan 11 Project Member

Issue description

Chrome Version       : Trunk
Other browsers tested:
  Add OK or FAIL after other browsers where you have tested this issue:
     Safari: PASS
    Firefox: PASS
    IE/Edge: didn't check

What steps will reproduce the problem?
1. Run on the console CSS.supports("margin: var(--foo) something absolutely pointless")
2. Run on the console CSS.supports("margin: someting-absolutely-pointless(var(--foo))")

What is the expected result?
Both return true, parse, and compute to invalid at computed-value time.

What happens instead of that?
First one correctly returns true, but second one does not. But I don't see what should make them behave differently.
 

Comment 1 by tabatkins@google.com, Jan 11

Status: Available (was: Unconfirmed)
Confirmed that both should be valid at parse time; the mere *presence* of a var() function in the tokens of a property automatically switch it to "assumed valid at parse time, check it after substitution" behavior.

Source: I wrote the spec.

Comment 2 by emilio@chromium.org, Jan 30

Status: Started (was: Available)
I'll just fix this, I don't really want to deal with compat issues due to this.

Comment 4 by emilio@chromium.org, Jan 30

Owner: emilio@chromium.org

Comment 5 by chris.he...@gmail.com, Jan 31

Both return false in current Edge and the same result as Chrome in the next version. So, fixing this in Chrome will make it work.

Comment 6 by dalecur...@chromium.org, Jan 31

Cc: beccahughes@chromium.org
More context from Twitter: bug was introduced while trying to fix  issue 864435  via 113cc2d54f97a1384ecc5c3a0b8bcddbd0c3765f

Comment 7 by dalecur...@chromium.org, Jan 31

Also jerem@ from the photos team said he'll look at fixing the original issue on their side.

Comment 8 by bugdroid, Jan 31

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/42eb4f28b94d798402c21c0bf239759efe150c95

commit 42eb4f28b94d798402c21c0bf239759efe150c95
Author: Emilio Cobos Álvarez <emilio@chromium.org>
Date: Thu Jan 31 18:56:45 2019

Don't skip invalid functions when looking for var references.

This matches the spec, WebKit and Firefox.

From https://drafts.csswg.org/css-variables/#using-variables:

> If a property contains one or more var() functions, and those functions are
> syntactically valid, the entire property’s grammar must be assumed to be
> valid at parse time. It is only syntax-checked at computed-value time, after
> var() functions have been substituted.

Bug:  921152 
Change-Id: Ie46268ffae4e01f457f379c674e0cf1a7ccea354
Reviewed-on: https://chromium-review.googlesource.com/c/1446653
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628023}
[modify] https://crrev.com/42eb4f28b94d798402c21c0bf239759efe150c95/third_party/blink/renderer/core/css/parser/css_variable_parser.cc
[delete] https://crrev.com/e20e1aa5b2a8bf600f2f165f63652e6ab465cdc6/third_party/blink/web_tests/css-parser/variables-invalid-functions.html
[modify] https://crrev.com/42eb4f28b94d798402c21c0bf239759efe150c95/third_party/blink/web_tests/external/wpt/css/css-env/supports-script.tentative.html
[modify] https://crrev.com/42eb4f28b94d798402c21c0bf239759efe150c95/third_party/blink/web_tests/external/wpt/css/cssom/CSS.html

Comment 9 by emilio@chromium.org, Feb 1

Status: Fixed (was: Started)

Sign in to add a comment