Skip to content

Commit 048781b

Browse files
committed
Bug 1713787 - Make custom properties that are IACVT guaranteed-invalid. r=boris
This effectively backs out bug 1623396. See: w3c/csswg-drafts#6006 https://drafts.csswg.org/css-variables/#guaranteed-invalid-value And related discussion. Matches Chrome stable as per https://groups.google.com/a/chromium.org/g/blink-dev/c/0xrbzYe_vxU/m/7bsL76n9CgAJ Depends on D116459 Differential Revision: https://phabricator.services.mozilla.com/D116460
1 parent 36c0f0d commit 048781b

File tree

4 files changed

+5
-35
lines changed

4 files changed

+5
-35
lines changed

servo/components/style/custom_properties.rs

+5-20
Original file line numberDiff line numberDiff line change
@@ -584,8 +584,7 @@ impl<'a> CustomPropertiesBuilder<'a> {
584584
match result {
585585
Ok(new_value) => new_value,
586586
Err(..) => {
587-
// Don't touch the map, this has the same effect as
588-
// making it compute to the inherited one.
587+
map.remove(name);
589588
return;
590589
},
591590
}
@@ -659,8 +658,7 @@ impl<'a> CustomPropertiesBuilder<'a> {
659658
None => return self.inherited.cloned(),
660659
};
661660
if self.may_have_cycles {
662-
let inherited = self.inherited.as_ref().map(|m| &***m);
663-
substitute_all(&mut map, inherited, self.device);
661+
substitute_all(&mut map, self.device);
664662
}
665663
map.shrink_to_fit();
666664
Some(Arc::new(map))
@@ -673,7 +671,6 @@ impl<'a> CustomPropertiesBuilder<'a> {
673671
/// It does cycle dependencies removal at the same time as substitution.
674672
fn substitute_all(
675673
custom_properties_map: &mut CustomPropertiesMap,
676-
inherited: Option<&CustomPropertiesMap>,
677674
device: &Device,
678675
) {
679676
// The cycle dependencies removal in this function is a variant
@@ -711,10 +708,7 @@ fn substitute_all(
711708
/// all unfinished strong connected components.
712709
stack: SmallVec<[usize; 5]>,
713710
map: &'a mut CustomPropertiesMap,
714-
/// The inherited variables. We may need to restore some if we fail
715-
/// substitution.
716-
inherited: Option<&'a CustomPropertiesMap>,
717-
/// to resolve the environment to substitute `env()` variables.
711+
/// To resolve the environment to substitute `env()` variables.
718712
device: &'a Device,
719713
}
720714

@@ -856,16 +850,8 @@ fn substitute_all(
856850
context.map.insert(name, computed_value);
857851
},
858852
Err(..) => {
859-
// This is invalid, reset it to the unset (inherited) value.
860-
let inherited = context.inherited.and_then(|m| m.get(&name)).cloned();
861-
match inherited {
862-
Some(computed_value) => {
863-
context.map.insert(name, computed_value);
864-
},
865-
None => {
866-
context.map.remove(&name);
867-
},
868-
};
853+
// This is invalid, reset it to the guaranteed-invalid value.
854+
context.map.remove(&name);
869855
},
870856
}
871857

@@ -883,7 +869,6 @@ fn substitute_all(
883869
stack: SmallVec::new(),
884870
var_info: SmallVec::new(),
885871
map: custom_properties_map,
886-
inherited,
887872
device,
888873
};
889874
traverse(name, &mut context);

testing/web-platform/meta/css/css-env/env-in-custom-properties.tentative.html.ini

-4
This file was deleted.

testing/web-platform/meta/css/css-variables/variable-substitution-variable-declaration.html.ini

-4
This file was deleted.

testing/web-platform/meta/css/css-variables/variables-substitute-guaranteed-invalid.html.ini

-7
This file was deleted.

0 commit comments

Comments
 (0)