Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-06 22:15:33 +01:00
Code Issues Projects Releases Packages Wiki Activity

style: Custom properties with invalid variable references should be unset, not invalid.

Browse source 
See https://github.com/w3c/csswg-drafts/issues/4075.

There are tests that will get updated and this will make pass in bug 1623347.

Differential Revision: https://phabricator.services.mozilla.com/D67373
This commit is contained in:
Emilio Cobos Álvarez 2020-03-26 11:34:12 +00:00
parent e0b4619fa5
commit 3004286308
1 changed files with 31 additions and 12 deletions
Download patch file Download diff file Expand all files Collapse all files

43
components/style/custom_properties.rs
View file

@ -579,9 +579,10 @@ impl<'a> CustomPropertiesBuilder<'a> {
match result { match result {
Ok(new_value) => Arc::new(new_value), Ok(new_value) => Arc::new(new_value),
Err(..) => { Err(..) => {
map.remove(name); // Don't touch the map, this has the same effect as
// making it compute to the inherited one.
return; return;
}, }
} }
} else { } else {
(*unparsed_value).clone() (*unparsed_value).clone()
@ -653,16 +654,22 @@ impl<'a> CustomPropertiesBuilder<'a> {
None => return self.inherited.cloned(), None => return self.inherited.cloned(),
}; };
if self.may_have_cycles { if self.may_have_cycles {
substitute_all(&mut map, self.device); let inherited = self.inherited.as_ref().map(|m| &***m);
substitute_all(&mut map, inherited, self.device);
} }
Some(Arc::new(map)) Some(Arc::new(map))
} }
} }
/// Resolve all custom properties to either substituted or invalid. /// Resolve all custom properties to either substituted, invalid, or unset
/// (meaning we should use the inherited value).
/// ///
/// It does cycle dependencies removal at the same time as substitution. /// It does cycle dependencies removal at the same time as substitution.
fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, device: &Device) { fn substitute_all(
custom_properties_map: &mut CustomPropertiesMap,
inherited: Option<&CustomPropertiesMap>,
device: &Device,
) {
// The cycle dependencies removal in this function is a variant // The cycle dependencies removal in this function is a variant
// of Tarjan's algorithm. It is mostly based on the pseudo-code // of Tarjan's algorithm. It is mostly based on the pseudo-code
// listed in // listed in
@ -698,6 +705,9 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, device: &Devi
/// all unfinished strong connected components. /// all unfinished strong connected components.
stack: SmallVec<[usize; 5]>, stack: SmallVec<[usize; 5]>,
map: &'a mut CustomPropertiesMap, 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, device: &'a Device,
} }
@ -831,18 +841,26 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, device: &Devi
return None; return None;
} }
// Now we have shown that this variable is not in a loop, and // Now we have shown that this variable is not in a loop, and all of its
// all of its dependencies should have been resolved. We can // dependencies should have been resolved. We can start substitution
// start substitution now. // now.
let result = substitute_references_in_value(&value, &context.map, &context.device); let result = substitute_references_in_value(&value, &context.map, &context.device);
match result { match result {
Ok(computed_value) => { Ok(computed_value) => {
context.map.insert(name, Arc::new(computed_value)); context.map.insert(name, Arc::new(computed_value));
}, }
Err(..) => { Err(..) => {
context.map.remove(&name); // 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);
},
};
}
} }
// All resolved, so return the signal value. // All resolved, so return the signal value.
@ -859,6 +877,7 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, device: &Devi
stack: SmallVec::new(), stack: SmallVec::new(),
var_info: SmallVec::new(), var_info: SmallVec::new(),
map: custom_properties_map, map: custom_properties_map,
inherited,
device, device,
}; };
traverse(name, &mut context); traverse(name, &mut context);