Auto merge of #18660 - upsuper:var-sub-inherit, r=emilio

Don't use inherited custom properties for substitution

<!-- Please describe your changes on the following line: -->
Custom properties from parent node has been considered in early stage of cascading them (in [`custom_properties::cascade`](01adcf69d7/components/style/custom_properties.rs (L461-L469))), and there is nothing in the spec stating that substitution should take inherited values into consideration when the value is invalid in the current node. Actually, there is a web-platform test explicitly checks that inherited value is not taken when the value is computed to invalid.

This would fix [bug 1402217](https://bugzilla.mozilla.org/show_bug.cgi?id=1402217).

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18660)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-09-28 09:06:48 -05:00 committed by GitHub
commit 2c73736f7d

View file

@ -503,7 +503,7 @@ pub fn finish_cascade(specified_values_map: Option<OrderedMap<&Name, BorrowedSpe
-> Option<Arc<CustomPropertiesMap>> {
if let Some(mut map) = specified_values_map {
remove_cycles(&mut map);
Some(Arc::new(substitute_all(map, inherited)))
Some(Arc::new(substitute_all(map)))
} else {
inherited.clone()
}
@ -555,8 +555,7 @@ fn remove_cycles(map: &mut OrderedMap<&Name, BorrowedSpecifiedValue>) {
}
/// Replace `var()` functions for all custom properties.
fn substitute_all(specified_values_map: OrderedMap<&Name, BorrowedSpecifiedValue>,
inherited: &Option<Arc<CustomPropertiesMap>>)
fn substitute_all(specified_values_map: OrderedMap<&Name, BorrowedSpecifiedValue>)
-> CustomPropertiesMap {
let mut custom_properties_map = CustomPropertiesMap::new();
let mut invalid = HashSet::new();
@ -566,7 +565,7 @@ fn substitute_all(specified_values_map: OrderedMap<&Name, BorrowedSpecifiedValue
// If this value is invalid at computed-time it wont be inserted in computed_values_map.
// Nothing else to do.
let _ = substitute_one(
name, value, &specified_values_map, inherited, None,
name, value, &specified_values_map, None,
&mut custom_properties_map, &mut invalid);
}
@ -580,7 +579,6 @@ fn substitute_all(specified_values_map: OrderedMap<&Name, BorrowedSpecifiedValue
fn substitute_one(name: &Name,
specified_value: &BorrowedSpecifiedValue,
specified_values_map: &OrderedMap<&Name, BorrowedSpecifiedValue>,
inherited: &Option<Arc<CustomPropertiesMap>>,
partial_computed_value: Option<&mut ComputedValue>,
custom_properties_map: &mut CustomPropertiesMap,
invalid: &mut HashSet<Name>)
@ -604,7 +602,7 @@ fn substitute_one(name: &Name,
&mut input, &mut position, &mut partial_computed_value,
&mut |name, partial_computed_value| {
if let Some(other_specified_value) = specified_values_map.get(&name) {
substitute_one(name, other_specified_value, specified_values_map, inherited,
substitute_one(name, other_specified_value, specified_values_map,
Some(partial_computed_value), custom_properties_map, invalid)
} else {
Err(())
@ -615,13 +613,8 @@ fn substitute_one(name: &Name,
partial_computed_value.push_from(position, &input, last_token_type);
partial_computed_value
} else {
// Invalid at computed-value time. Use the inherited value.
if let Some(inherited_value) = inherited.as_ref().and_then(|i| i.values.get(name)) {
inherited_value.clone()
} else {
invalid.insert(name.clone());
return Err(())
}
invalid.insert(name.clone());
return Err(())
}
} else {
// The specified value contains no var() reference