servo: Merge #15702 - Invalid three value positions are no longer accepted (from shubDhond:master); r=upsuper
authorshubDhond <shubham.dhond@mail.utoronto.ca>
Sat, 04 Mar 2017 03:11:44 -0800
changeset 374959 07d2b09b4f76040c9a43e1c63067b2ae27497944
parent 374958 d3fb96a2c6ef0d08a1037d3503882eea9fd2ecb5
child 374960 97e94e028ed4d0cd64be28d1eb3cbf3106f051aa
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersupsuper
bugs15702, 15488
milestone54.0a1
servo: Merge #15702 - Invalid three value positions are no longer accepted (from shubDhond:master); r=upsuper <!-- Please describe your changes on the following line: --> - Position parse no longer accepts invalid three value positions --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #15488 . <!-- Either: --> - [X] There are tests for these changes OR <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 8c4931f26f49cf328b63880b6b9b9bf8d99279c4
servo/components/style/values/specified/position.rs
servo/tests/unit/style/parsing/basic_shape.rs
servo/tests/unit/style/parsing/position.rs
--- a/servo/components/style/values/specified/position.rs
+++ b/servo/components/style/values/specified/position.rs
@@ -182,23 +182,21 @@ impl Parse for Position {
         // Try to parse third and fourth values
         if let Ok(third) = input.try(|i| PositionComponent::parse(context, i)) {
             if let Ok(fourth) = input.try(|i| PositionComponent::parse(context, i)) {
                 // Handle 4 value background position
                 Position::new(Some(second), Some(fourth), Some(first), Some(third))
             } else {
                 // Handle 3 value background position there are several options:
                 if let PositionCategory::LengthOrPercentage = category(&first) {
-                    // "length keyword length"
-                    Position::new(Some(first), Some(third), None, Some(second))
+                    Err(())
                 } else {
                     if let PositionCategory::LengthOrPercentage = category(&second) {
                         if let PositionCategory::LengthOrPercentage = category(&third) {
-                            // "keyword length length"
-                            Position::new(Some(second), Some(third), Some(first), None)
+                            Err(())
                         } else {
                             // "keyword length keyword"
                             Position::new(Some(second), None, Some(first), Some(third))
                         }
                     } else {
                         // "keyword keyword length"
                         Position::new(None, Some(third), Some(first), Some(second))
                     }
--- a/servo/tests/unit/style/parsing/basic_shape.rs
+++ b/servo/tests/unit/style/parsing/basic_shape.rs
@@ -107,19 +107,18 @@ fn test_circle() {
     assert_roundtrip_basicshape!(Circle::parse, "circle(at bottom 5px right 10px)",
                                                 "circle(at right 10px bottom 5px)");
     assert_roundtrip_basicshape!(Circle::parse, "circle(at right 5% top 0px)",
                                                 "circle(at 95% 0%)");
     assert_roundtrip_basicshape!(Circle::parse, "circle(at right 5% bottom 0px)",
                                                 "circle(at 95% 100%)");
     assert_roundtrip_basicshape!(Circle::parse, "circle(at right 5% bottom 1px)",
                                                 "circle(at right 5% bottom 1px)");
-    assert_roundtrip_basicshape!(Circle::parse, "circle(at 5% bottom 1px)",
-                                                "circle(at left 5% bottom 1px)");
 
+    assert!(parse(Circle::parse, "circle(at 5% bottom 1px)").is_err());
     assert!(parse(Circle::parse, "circle(at top 40%)").is_err());
     assert!(parse(Circle::parse, "circle(-10px)").is_err());
 }
 
 #[test]
 fn test_ellipse() {
     assert_roundtrip_basicshape!(Ellipse::parse, "ellipse(at center)", "ellipse(at 50% 50%)");
     assert_roundtrip_basicshape!(Ellipse::parse, "ellipse()", "ellipse(at 50% 50%)");
--- a/servo/tests/unit/style/parsing/position.rs
+++ b/servo/tests/unit/style/parsing/position.rs
@@ -43,16 +43,21 @@ fn test_position() {
     assert_roundtrip_with_context!(Position::parse, "top 15px left", "left top 15px");
     assert_roundtrip_with_context!(Position::parse, "left 10px top", "left 10px top");
     assert_roundtrip_with_context!(Position::parse, "top left 10px", "left 10px top");
     assert_roundtrip_with_context!(Position::parse, "right 10px bottom", "right 10px bottom");
     assert_roundtrip_with_context!(Position::parse, "bottom right 10px", "right 10px bottom");
     assert_roundtrip_with_context!(Position::parse, "center right 10px", "right 10px center");
     assert_roundtrip_with_context!(Position::parse, "center bottom 10px", "center bottom 10px");
 
+    // Invalid 3 value positions
+    assert!(parse(Position::parse, "20px 30px 20px").is_err());
+    assert!(parse(Position::parse, "top 30px 20px").is_err());
+    assert!(parse(Position::parse, "50% bottom 20%").is_err());
+
     // Only horizontal and vertical keywords can have positions
     assert!(parse(Position::parse, "center 10px left 15px").is_err());
     assert!(parse(Position::parse, "center 10px 15px").is_err());
     assert!(parse(Position::parse, "center 10px bottom").is_err());
 
     // "Horizontal Horizontal" or "Vertical Vertical" positions cause error
     assert!(parse(Position::parse, "left right").is_err());
     assert!(parse(Position::parse, "left 10px right").is_err());