From 33ad82c3da6e2aabca811dae30ee3f5b6a05abf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 2 Jun 2023 01:58:40 +0200 Subject: [PATCH] style: Make custom properties that are IACVT guaranteed-invalid This effectively backs out bug 1623396. See: https://github.com/w3c/csswg-drafts/pull/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 --- components/style/custom_properties.rs | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index 96328d9a51a..641a421bd5c 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -597,8 +597,7 @@ impl<'a> CustomPropertiesBuilder<'a> { match result { Ok(new_value) => new_value, Err(..) => { - // Don't touch the map, this has the same effect as - // making it compute to the inherited one. + map.remove(name); return; }, } @@ -672,8 +671,7 @@ impl<'a> CustomPropertiesBuilder<'a> { None => return self.inherited.cloned(), }; if self.may_have_cycles { - let inherited = self.inherited.as_ref().map(|m| &***m); - substitute_all(&mut map, inherited, self.device); + substitute_all(&mut map, self.device); } map.shrink_to_fit(); Some(Arc::new(map)) @@ -686,7 +684,6 @@ impl<'a> CustomPropertiesBuilder<'a> { /// It does cycle dependencies removal at the same time as substitution. fn substitute_all( custom_properties_map: &mut CustomPropertiesMap, - inherited: Option<&CustomPropertiesMap>, device: &Device, ) { // The cycle dependencies removal in this function is a variant @@ -724,10 +721,7 @@ fn substitute_all( /// all unfinished strong connected components. stack: SmallVec<[usize; 5]>, map: &'a mut CustomPropertiesMap, - /// The inherited variables. We may need to restore some if we fail - /// substitution. - inherited: Option<&'a CustomPropertiesMap>, - /// to resolve the environment to substitute `env()` variables. + /// To resolve the environment to substitute `env()` variables. device: &'a Device, } @@ -869,16 +863,8 @@ fn substitute_all( context.map.insert(name, computed_value); }, Err(..) => { - // This is invalid, reset it to the unset (inherited) value. - let inherited = context.inherited.and_then(|m| m.get(&name)).cloned(); - match inherited { - Some(computed_value) => { - context.map.insert(name, computed_value); - }, - None => { - context.map.remove(&name); - }, - }; + // This is invalid, reset it to the guaranteed-invalid value. + context.map.remove(&name); }, } @@ -896,7 +882,6 @@ fn substitute_all( stack: SmallVec::new(), var_info: SmallVec::new(), map: custom_properties_map, - inherited, device, }; traverse(name, &mut context);