Bug 1709018 - Don't simplify percentages that resolve to lengths in min/max/clamp. r=boris

Those can't be ordered at specified / computed value time, since the
percentage basis could be negative.

Needs tests of course, running through try atm.

Differential Revision: https://phabricator.services.mozilla.com/D115591

servo/components/style/values/computed/length_percentage.rs

@@ -607,22 +607,11 @@ impl CalcLengthPercentageLeaf {
 impl PartialOrd for CalcLengthPercentageLeaf {
     fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
         use self::CalcLengthPercentageLeaf::*;
-
-        if std::mem::discriminant(self) != std::mem::discriminant(other) {
-            return None;
-        }
-
+        // NOTE: Percentages can't be compared reasonably here because the
+        // percentage basis might be negative, see bug 1709018.
         match (self, other) {
             (&Length(ref one), &Length(ref other)) => one.partial_cmp(other),
-            (&Percentage(ref one), &Percentage(ref other)) => one.partial_cmp(other),
-            _ => {
-                match *self {
-                    Length(..) | Percentage(..) => {},
-                }
-                unsafe {
-                    debug_unreachable!("Forgot a branch?");
-                }
-            },
+            _ => None,
         }
     }
 }

servo/components/style/values/specified/calc.rs

@@ -107,8 +107,12 @@ impl PartialOrd for Leaf {
         }
 
         match (self, other) {
+            // NOTE: Percentages can't be compared reasonably here because the
+            // percentage basis might be negative, see bug 1709018.
+            // Conveniently, we only use this for <length-percentage> (for raw
+            // percentages, we go through resolve()).
+            (&Percentage(..), &Percentage(..)) => None,
             (&Length(ref one), &Length(ref other)) => one.partial_cmp(other),
-            (&Percentage(ref one), &Percentage(ref other)) => one.partial_cmp(other),
             (&Angle(ref one), &Angle(ref other)) => one.degrees().partial_cmp(&other.degrees()),
             (&Time(ref one), &Time(ref other)) => one.seconds().partial_cmp(&other.seconds()),
             (&Number(ref one), &Number(ref other)) => one.partial_cmp(other),

testing/web-platform/meta/css/css-backgrounds/background-position-negative-percentage-comparison.html.ini

@@ -1,2 +0,0 @@
-[background-position-negative-percentage-comparison.html]
-  expected: FAIL

testing/web-platform/meta/css/css-values/minmax-percentage-serialize.html.ini

@@ -1,31 +0,0 @@
-[minmax-percentage-serialize.html]
-  ['max(1%, 2%, 3%)' as a computed value should serialize as 'max(1%, 2%, 3%)'.]
-    expected: FAIL
-
-  ['min(3%, 2%, 1%)' as a specified value should serialize as 'min(3%, 2%, 1%)'.]
-    expected: FAIL
-
-  ['max(3%, 2%, 1%)' as a specified value should serialize as 'max(3%, 2%, 1%)'.]
-    expected: FAIL
-
-  ['min(1%, 2%, 3%)' as a computed value should serialize as 'min(1%, 2%, 3%)'.]
-    expected: FAIL
-
-  ['min(1%, 2%, 3%)' as a specified value should serialize as 'min(1%, 2%, 3%)'.]
-    expected: FAIL
-
-  ['max(3%, 2%, 1%)' as a computed value should serialize as 'max(3%, 2%, 1%)'.]
-    expected: FAIL
-
-  ['min(3%, 2%, 1%)' as a computed value should serialize as 'min(3%, 2%, 1%)'.]
-    expected: FAIL
-
-  ['max(1%, 2%, 3%)' as a specified value should serialize as 'max(1%, 2%, 3%)'.]
-    expected: FAIL
-
-  ['min(1%, 2%, 3%) 0px' as a specified value should serialize as 'min(1%, 2%, 3%) 0px'.]
-    expected: FAIL
-
-  ['min(1%, 2%, 3%) 0px' as a computed value should serialize as 'min(1%, 2%, 3%) 0px'.]
-    expected: FAIL
-

testing/web-platform/tests/css/css-values/minmax-percentage-serialize.html

@@ -65,8 +65,8 @@ test_serialization(
 
 test_serialization(
     'calc(min(1%, 2%) + max(3%, 4%) + 10%)',
-    'calc(15%)',
-    '15%',
+    'calc(10% + min(1%, 2%) + max(3%, 4%))',
+    'calc(10% + min(1%, 2%) + max(3%, 4%))',
     '15px');
 
 </script>