Implement Media Queries Level 4

[luniv]

Spec: http://drafts.csswg.org/mediaqueries/#media-types

[hoppipolla-critic-bot]

Critic review: https://critic.hoppipolla.co.uk/r/4294

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

[jdm]

Not going to lie, this is a pretty fantastic PR.

[pcwalton]

Whoa.

[SimonSapin]

This looks great! I’ll take the review (on Critic).

One thing is that the MQ4 spec (as opposed to 3) has not been reviewed a lot yet, and so our implementation should probably be behind the --experimental flag (whose presence can be tested with ::util::opts::experimental_enabled()) since the spec might still change significantly. When the flag is not given, we should only support level 3 syntax and features.

I’ll leave it to you to decide how to do that: either have conditional code paths in the new code or restore the old code (media_queries.rs)

By the way, as the Critic bot said, please avoid rebasing, rewriting, or squashing already-pushed commits until the review is done, as that makes it hard for the review tool to keep tracking comments made on specific diff lines.

[luniv]

I've added two new commits that:

• through a lot of macro magic, allow media features to be defined via a DSL resembling the MQ 4 spec; e.g.

media_features! {
    // MQ 4 § 4. Screen/Device Dimensions
    // MQ 4 § 4.1
    Width(name: "width",
          value: Length,
          type: range,
          availability: {
              since: SpecificationLevel::MEDIAQ3
          },
          impl: {
              context: Au,
              zero: Au(0),
              compute: compute_length
          }),
    ...
    // MQ 4 § 5.4
    UpdateFrequency(name: "update-frequency",
                    value: {
                        "none" => None,
                        "slow" => Slow,
                        "normal" => Normal
                    },
                    type: discrete,
                    availability: {
                        since: SpecificationLevel::MEDIAQ4
                    }),
    ...
}

• Specification level support, with some MQ 3 specifics (such as optional whitespace between 'and', allowed media types, etc...) I'm not sure though if I've completely captured all the differences.

[jdm]

😍

[metajack]

@SimonSapin How goes the review? It appears at 0% in Critic.

[SimonSapin]

@luniv Sorry for the delays. I’ve submitted a partial review on Critic, but:

While it’s very cool to have a MQ4 implementation for Servo, it has very little practical impact in the near term. The MQ4 spec itself is not very stable yet, and no shipping browser implements it. (It’s not even listed on caniuse or MDN.) So I’m confident that practically no website relies on it. There’s also a risk that the spec still changes significantly from under us.

I’m really grateful for this work you did, but this is why it’s relatively low priority and it may be a while still before I finish the review.

[luniv]

Fair enough.

Would it make sense to split out the parts that are in MQ3 (e.g. the other
features) as a separate PR?

On Mon, Apr 20, 2015 at 10:14 AM, Simon Sapin notifications@github.com
wrote:

@luniv https://github.com/luniv Sorry for the delays. I’ve submitted a
partial review on Critic, but:

While it’s very cool to have a MQ4 implementation for Servo, it has very
little practical impact in the near term. The MQ4 spec itself is not very
stable yet, and no shipping browser implements it. (It’s not even listed on
caniuse http://caniuse.com/#search=media%20queries or MDN
https://developer.mozilla.org/en-US/docs/Web/Guide/CSS/Media_queries#Browser_compatibility.)
So I’m confident that practically no website relies on it. There’s also a
risk that the spec still changes significantly from under us.

I’m really grateful for this work you did, but this is why it’s relatively
low priority and it may be a while still before I finish the review.

—
Reply to this email directly or view it on GitHub
#5265 (comment).

[SimonSapin]

One of my review comments is that I’d rather not parse at all media features for which we don’t have any meaningful support. (Those that have TODO in your PR.)

But yes, adding meaningful support for more level 3 media features would be good.

[metajack]

@luniv Still hacking on this?

[luniv]

I was, but I’ve relocated internationally, so haven’t had time to work on it.

I’ll close this PR and revisit it when the dust from moving settles.

On Jul 29, 2015, at 12:56 PM, Jack Moffitt notifications@github.com wrote:

@luniv https://github.com/luniv Still hacking on this?

—
Reply to this email directly or view it on GitHub #5265 (comment).