Bug 1460655: Support x as a resolution unit. r=xidorn draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 10 May 2018 18:11:52 +0200
changeset 794249 dc1cdb8f98e6aaf62ed46bfcf7b2c7fdaebdc189
parent 794248 6a99802f307ad0a331272bac80029d334987a88b
push id109629
push userbmo:emilio@crisal.io
push dateFri, 11 May 2018 16:31:53 +0000
reviewersxidorn
bugs1460655
milestone62.0a1
Bug 1460655: Support x as a resolution unit. r=xidorn MozReview-Commit-ID: TjU0FLCLMN
layout/style/test/test_media_queries.html
servo/components/style/values/specified/resolution.rs
testing/web-platform/tests/css/mediaqueries/test_media_queries.html
--- a/layout/style/test/test_media_queries.html
+++ b/layout/style/test/test_media_queries.html
@@ -547,21 +547,24 @@ function run() {
   for (i in features) {
     feature = features[i];
     expression_should_be_parseable(feature + ": 3dpi");
     expression_should_be_parseable(feature + ":3dpi");
     expression_should_be_parseable(feature + ": 3.0dpi");
     expression_should_be_parseable(feature + ": 3.4dpi");
     expression_should_be_parseable(feature + "\t: 120dpcm");
     expression_should_be_parseable(feature + ": 1dppx");
+    expression_should_be_parseable(feature + ": 1x");
     expression_should_be_parseable(feature + ": 1.5dppx");
+    expression_should_be_parseable(feature + ": 1.5x");
     expression_should_be_parseable(feature + ": 2.0dppx");
     expression_should_not_be_parseable(feature + ": 0dpi");
     expression_should_not_be_parseable(feature + ": -3dpi");
     expression_should_not_be_parseable(feature + ": 0dppx");
+    expression_should_not_be_parseable(feature + ": 0x");
   }
 
   // Find the resolution using max-resolution
   var resolution = 0;
   do {
     ++resolution;
     if (resolution > 10000) {
       ok(false, "resolution greater than 10000dpi???");
@@ -571,16 +574,17 @@ function run() {
 
   // resolution should now be Math.ceil() of the actual resolution.
   var dpi_high;
   var dpi_low = resolution - 1;
   if (query_applies("(min-resolution: " + resolution + "dpi)")) {
     // It's exact!
     should_apply("(resolution: " + resolution + "dpi)");
     should_apply("(resolution: " + Math.floor(resolution/96) + "dppx)");
+    should_apply("(resolution: " + Math.floor(resolution/96) + "x)");
     should_not_apply("(resolution: " + (resolution + 1) + "dpi)");
     should_not_apply("(resolution: " + (resolution - 1) + "dpi)");
     dpi_high = resolution + 1;
   } else {
     // We have no way to test resolution applying since it need not be
     // an integer.
     should_not_apply("(resolution: " + resolution + "dpi)");
     should_not_apply("(resolution: " + (resolution - 1) + "dpi)");
--- a/servo/components/style/values/specified/resolution.rs
+++ b/servo/components/style/values/specified/resolution.rs
@@ -12,37 +12,42 @@ use style_traits::{ParseError, StylePars
 use values::CSSFloat;
 
 /// A specified resolution.
 #[derive(Clone, Debug, PartialEq, ToCss)]
 pub enum Resolution {
     /// Dots per inch.
     #[css(dimension)]
     Dpi(CSSFloat),
+    /// An alias unit for dots per pixel.
+    #[css(dimension)]
+    X(CSSFloat),
     /// Dots per pixel.
     #[css(dimension)]
     Dppx(CSSFloat),
     /// Dots per centimeter.
     #[css(dimension)]
     Dpcm(CSSFloat),
 }
 
 impl Resolution {
     /// Convert this resolution value to dppx units.
     pub fn to_dppx(&self) -> CSSFloat {
         match *self {
+            Resolution::X(f) |
             Resolution::Dppx(f) => f,
             _ => self.to_dpi() / 96.0,
         }
     }
 
     /// Convert this resolution value to dpi units.
     pub fn to_dpi(&self) -> CSSFloat {
         match *self {
             Resolution::Dpi(f) => f,
+            Resolution::X(f) |
             Resolution::Dppx(f) => f * 96.0,
             Resolution::Dpcm(f) => f * 2.54,
         }
     }
 }
 
 impl Parse for Resolution {
     fn parse<'i, 't>(
@@ -60,14 +65,15 @@ impl Parse for Resolution {
         if value <= 0. {
             return Err(location.new_custom_error(StyleParseErrorKind::UnspecifiedError));
         }
 
         match_ignore_ascii_case! { &unit,
             "dpi" => Ok(Resolution::Dpi(value)),
             "dppx" => Ok(Resolution::Dppx(value)),
             "dpcm" => Ok(Resolution::Dpcm(value)),
+            "x" => Ok(Resolution::X(value)),
             _ => Err(location.new_custom_error(
                 StyleParseErrorKind::UnexpectedDimension(unit.clone())
             )),
         }
     }
 }
--- a/testing/web-platform/tests/css/mediaqueries/test_media_queries.html
+++ b/testing/web-platform/tests/css/mediaqueries/test_media_queries.html
@@ -343,18 +343,26 @@ function run() {
     features = [ "resolution", "min-resolution", "max-resolution" ];
     for (i in features) {
       feature = features[i];
       expression_should_be_parseable(feature + ": 3dpi");
       expression_should_be_parseable(feature + ":3dpi");
       expression_should_be_parseable(feature + ": 3.0dpi");
       expression_should_be_parseable(feature + ": 3.4dpi");
       expression_should_be_parseable(feature + "\t: 120dpcm");
+      expression_should_be_parseable(feature + ": 1dppx");
+      expression_should_be_parseable(feature + ": 1x");
+      expression_should_be_parseable(feature + ": 1.5dppx");
+      expression_should_be_parseable(feature + ": 1.5x");
+      expression_should_be_parseable(feature + ": 2.0dppx");
+      // TODO(emilio): Doesn't seem right to exclude 0 here.
       expression_should_not_be_parseable(feature + ": 0dpi");
       expression_should_not_be_parseable(feature + ": -3dpi");
+      expression_should_not_be_parseable(feature + ": 0dppx");
+      expression_should_not_be_parseable(feature + ": 0x");
     }
 
     // Find the resolution using max-resolution
     var resolution = 0;
     do {
       ++resolution;
       if (resolution > 10000) {
         break;
@@ -365,16 +373,18 @@ function run() {
     }, "find_resolution");
 
     // resolution should now be Math.ceil() of the actual resolution.
     var dpi_high;
     var dpi_low = resolution - 1;
     if (query_applies("(min-resolution: " + resolution + "dpi)")) {
       // It's exact!
       should_apply("(resolution: " + resolution + "dpi)");
+      should_apply("(resolution: " + Math.floor(resolution/96) + "dppx)");
+      should_apply("(resolution: " + Math.floor(resolution/96) + "x)");
       should_not_apply("(resolution: " + (resolution + 1) + "dpi)");
       should_not_apply("(resolution: " + (resolution - 1) + "dpi)");
       dpi_high = resolution + 1;
     } else {
 	  // We have no way to test resolution applying since it need not be
 	  // an integer.
       should_not_apply("(resolution: " + resolution + "dpi)");
       should_not_apply("(resolution: " + (resolution - 1) + "dpi)");