Support @supports #14789

Manishearth · 2016-12-30T06:29:03Z

fixes #14786

cc @heycam @upsuper
r? @SimonSapin

This change is

highfive · 2016-12-30T06:29:07Z

Heads up! This PR modifies the following files:

@bholley: components/style/supports.rs, components/style/lib.rs, components/style/stylesheets.rs, components/style/stylist.rs
@emilio: components/style/supports.rs, components/style/lib.rs, components/style/stylesheets.rs, components/style/stylist.rs

highfive · 2016-12-30T06:29:08Z

Warning

These commits modify style code, but no tests are modified. Please consider adding a test!

Manishearth · 2016-12-30T07:39:19Z

components/style/supports.rs

+
+        if let Ok(first_cond) = input.try(SupportsCondition::parse_with_parens) {
+            if input.is_exhausted() {
+                // nested parens


This is sort of wrong. Spec says we must preserve parens for serialization. Such rules should either be represented as a Single(Box<SupportsRule>) or just an And rule with a single element. Not sure which I should use.

Made it explicitly handle parenthesized cases, which fixes a couple tests.

@heycam suggested never storing it in a structured form in the first place, since we only need it for ToCssing -- we can just evaluate whilst parsing. However, the spec suggests we also clean up the whitespace and such, and it's better to store it in the structured form for that.

(Also, I bet eventually there will be more structured CSSOM access to this like we have for media queries)

Manishearth · 2016-12-30T08:04:25Z

@bors-servo try

bors-servo · 2016-12-30T08:04:29Z

⌛ Trying commit af4d5d6 with merge 9e93ba5...


        Auto merge of #14789 - Manishearth:supports, r=<try>

@heycam

Support @supports fixes #14786 cc @heycam @upsuper r? @SimonSapin  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14789)

bors-servo · 2016-12-30T08:18:28Z

💔 Test failed - linux-dev

Manishearth · 2016-12-30T08:21:28Z

wpt at http://build.servo.org/builders/linux-rel-wpt/builds/1637

css at http://build.servo.org/builders/linux-rel-css/builds/1630

looks like geckolib hasn't been rustupped yet. Not sure if I should.

Manishearth · 2016-12-30T09:08:47Z

@bors-servo try

bors-servo · 2016-12-30T09:08:51Z

⌛ Trying commit 67c2ae6 with merge 7ae880f...


        Auto merge of #14789 - Manishearth:supports, r=<try>

@heycam

Support @supports fixes #14786 cc @heycam @upsuper r? @SimonSapin  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14789)

bors-servo · 2016-12-30T09:23:54Z

💔 Test failed - linux-dev

Manishearth · 2016-12-30T09:32:45Z

wpt: http://build.servo.org/builders/linux-rel-wpt/builds/1638
css: http://build.servo.org/builders/linux-rel-css/builds/1631

emilio · 2016-12-30T11:02:10Z

components/style/stylesheets.rs

@@ -859,6 +892,10 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> {
                let media_queries = parse_media_query_list(input);
                Ok(AtRuleType::WithBlock(AtRulePrelude::Media(Arc::new(RwLock::new(media_queries)))))
            },
+            "supports" => {
+                let cond = SupportsCondition::parse(input, true)?;


Is ? in stable rust? Can we use it in stylo?

Yes, but stylo's Rust isn't new enough.

emilio · 2016-12-30T11:02:10Z

components/style/supports.rs

+        match *self {
+            SupportsCondition::Not(ref cond) => !cond.eval(cx),
+            SupportsCondition::And(ref vec) => vec.iter().all(|c| c.eval(cx)),
+            SupportsCondition::Or(ref vec) => !vec.iter().all(|c| !c.eval(cx)),


vec.iter().any().(|c| c.eval(cx))?

emilio · 2016-12-30T11:02:10Z

components/style/supports.rs

+            ExperimentalProperty => false, // only happens for experimental props
+                                           // that haven't been enabled
+            InvalidValue => false,
+            AnimationPropertyInKeyframeBlock => false, // should never happen


Mark as unreachable!.

bholley · 2016-12-30T20:18:51Z

Bumping rust-stable-version to 1.13 should be fine - the Gecko builders were updated in https://bugzilla.mozilla.org/show_bug.cgi?id=1316751

Manishearth · 2016-12-31T00:16:30Z

@bors-servo try

bors-servo · 2016-12-31T00:16:33Z

⌛ Trying commit db3e018 with merge 865242d...


        Auto merge of #14789 - Manishearth:supports, r=<try>

@heycam

Support @supports fixes #14786 cc @heycam @upsuper r? @SimonSapin  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14789)

Manishearth · 2017-01-06T00:39:55Z

Updated, fixed tests. Thanks for the fixups!

bors-servo · 2017-01-08T07:16:03Z

☔ The latest upstream changes (presumably #14712) made this pull request unmergeable. Please resolve the merge conflicts.


        Support @supports (fixes #14786)


         CSSOM bits for @supports: CSSConditionRule and CSSSupportsRule, with…

… conditionText attribute


        Update test expectations, enable css-conditional-3 tests


        Add CSS.supports()


        Add test for CSS interface

Manishearth · 2017-01-09T14:56:00Z

@bors-servo try

bors-servo · 2017-01-09T14:56:04Z

⌛ Trying commit dbc35f4 with merge 1fc9b6b...


        Auto merge of #14789 - Manishearth:supports, r=<try>

@heycam

Support @supports fixes #14786 cc @heycam @upsuper r? @SimonSapin  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14789)

bors-servo · 2017-01-09T16:34:04Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-gnu-dev, windows-msvc-dev


        Fix too many parens in @supports serialization.


        mach test-unit: also pass extra arguments to style_tests harness

Manishearth · 2017-01-09T18:41:36Z

@bors-servo r=SimonSapin

bors-servo · 2017-01-09T18:41:37Z

📌 Commit 93f66ab has been approved by SimonSapin

SimonSapin · 2017-01-09T18:41:48Z

Reviewed 1 of 96 files at r15, 5 of 96 files at r20, 12 of 12 files at r21, 74 of 74 files at r22, 3 of 3 files at r23, 2 of 2 files at r24, 2 of 93 files at r25, 12 of 12 files at r26, 74 of 74 files at r27, 3 of 3 files at r28, 2 of 2 files at r29.
Review status: all files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.

Comments from Reviewable

bors-servo · 2017-01-09T18:54:40Z

⌛ Testing commit 93f66ab with merge 50bba77...


        Auto merge of #14789 - Manishearth:supports, r=SimonSapin

@heycam

Support @supports fixes #14786 cc @heycam @upsuper r? @SimonSapin  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14789)

bors-servo · 2017-01-09T19:56:36Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-gnu-dev, windows-msvc-dev

highfive assigned SimonSapin Dec 30, 2016

highfive added the S-awaiting-review label Dec 30, 2016

Manishearth force-pushed the Manishearth:supports branch from 06f4544 to af4d5d6 Dec 30, 2016

Manishearth reviewed Dec 30, 2016

View changes

Manishearth changed the title ~~Support @supports (fixes #14786)~~ Support @supports Dec 30, 2016

highfive added the S-tests-failed label Dec 30, 2016

Manishearth force-pushed the Manishearth:supports branch from af4d5d6 to 68abf3d Dec 30, 2016

highfive removed the S-tests-failed label Dec 30, 2016

Manishearth force-pushed the Manishearth:supports branch from 68abf3d to 67c2ae6 Dec 30, 2016

highfive added the S-tests-failed label Dec 30, 2016

emilio reviewed Dec 30, 2016

View changes

Manishearth force-pushed the Manishearth:supports branch from 67c2ae6 to 070283f Dec 30, 2016

highfive removed the S-tests-failed label Dec 30, 2016

Manishearth force-pushed the Manishearth:supports branch from 070283f to db3e018 Dec 31, 2016

highfive added the S-needs-rebase label Jan 8, 2017

Manishearth added 5 commits Dec 30, 2016

Support @supports (fixes #14786)

1b0842e

CSSOM bits for @supports: CSSConditionRule and CSSSupportsRule, with…

b4a83b6

… conditionText attribute

Update test expectations, enable css-conditional-3 tests

56731dc

Add CSS.supports()

62bea28

Add test for CSS interface

Loading status checks…

dbc35f4

Manishearth force-pushed the Manishearth:supports branch from 0485864 to dbc35f4 Jan 8, 2017

SimonSapin added 2 commits Jan 9, 2017

Fix too many parens in @supports serialization.

b5d9210

mach test-unit: also pass extra arguments to style_tests harness

Loading status checks…

93f66ab

highfive assigned SimonSapin and unassigned mbrubeck Jan 9, 2017

highfive added S-awaiting-merge and removed S-awaiting-review S-needs-rebase labels Jan 9, 2017

highfive removed the S-awaiting-merge label Jan 9, 2017

bors-servo mentioned this pull request Jan 9, 2017

Enable more CSS fonts tests #11311

Closed

Manishearth deleted the Manishearth:supports branch May 7, 2019

servo / servo

Support @supports #14789

Support @supports #14789

Manishearth commented Dec 30, 2016 •

edited

highfive commented Dec 30, 2016

highfive commented Dec 30, 2016

This comment has been minimized.

This comment has been minimized.

Manishearth commented Dec 30, 2016

bors-servo commented Dec 30, 2016

bors-servo commented Dec 30, 2016

Manishearth commented Dec 30, 2016

Manishearth commented Dec 30, 2016

bors-servo commented Dec 30, 2016

bors-servo commented Dec 30, 2016

Manishearth commented Dec 30, 2016

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bholley commented Dec 30, 2016

Manishearth commented Dec 31, 2016

bors-servo commented Dec 31, 2016

Manishearth commented Jan 6, 2017

bors-servo commented Jan 8, 2017

Manishearth commented Jan 9, 2017

bors-servo commented Jan 9, 2017

bors-servo commented Jan 9, 2017

Manishearth commented Jan 9, 2017

bors-servo commented Jan 9, 2017

SimonSapin commented Jan 9, 2017

bors-servo commented Jan 9, 2017

bors-servo commented Jan 9, 2017

servo / servo

Join GitHub today

Support @supports #14789

Support @supports #14789

Conversation

Manishearth commented Dec 30, 2016 • edited

highfive commented Dec 30, 2016

highfive commented Dec 30, 2016

This comment has been minimized.

Manishearth Dec 30, 2016

This comment has been minimized.

Manishearth Dec 31, 2016

Manishearth commented Dec 30, 2016

bors-servo commented Dec 30, 2016

bors-servo commented Dec 30, 2016

Manishearth commented Dec 30, 2016

Manishearth commented Dec 30, 2016

bors-servo commented Dec 30, 2016

bors-servo commented Dec 30, 2016

Manishearth commented Dec 30, 2016

This comment has been minimized.

emilio Dec 30, 2016

This comment has been minimized.

Manishearth Dec 30, 2016

This comment has been minimized.

emilio Dec 30, 2016 • edited

This comment has been minimized.

emilio Dec 30, 2016

bholley commented Dec 30, 2016

Manishearth commented Dec 31, 2016

bors-servo commented Dec 31, 2016

Manishearth commented Jan 6, 2017

bors-servo commented Jan 8, 2017

Manishearth commented Jan 9, 2017

bors-servo commented Jan 9, 2017

bors-servo commented Jan 9, 2017

Manishearth commented Jan 9, 2017

bors-servo commented Jan 9, 2017

SimonSapin commented Jan 9, 2017

bors-servo commented Jan 9, 2017

bors-servo commented Jan 9, 2017

Manishearth commented Dec 30, 2016 •

edited

emilio Dec 30, 2016 •

edited