Add a general test to make sure we return the correct change hint for all css properties in CalcDifference()
Categories
(Core :: CSS Parsing and Computation, enhancement, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox68 | --- | fixed |
People
(Reporter: boris, Assigned: emilio)
Details
Attachments
(3 files)
When adding a new css properties, we have to make sure CalcDifference()
return the correct change hint, as least nsChangeHint_NeutralChange
if any of the css property is changed.
For example, we change offset-distance
in an outer element. If the inner element inherits the value, it should also be changed:
<div id="outer" style="offset-distance: 10px">
<div id="inner" style="offset-distance: inherit"></div>
</div>
<script>
assert_equals(getComputedStyle(outer).offsetDistance, "10px");
assert_equals(getComputedStyle(inner).offsetDistance, "10px");
outer.style.offsetDistance = "30px";
assert_equals(getComputedStyle(outer).offsetDistance, "30px");
assert_equals(getComputedStyle(inner).offsetDistance, "30px"); // this should pass if we return NeutralChange.
</script>
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Comment 1•5 years ago
|
||
Note: The current try doesn't catch this if someone or I forget to add this change hint.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 2•5 years ago
|
||
It was missing the cases where you changed values, but not count, and the image
was not visible, like:
mask-image: none;
mask-mode: match-source, match-source;
Then change mask-mode to match-source, alpha
, for example.
Assignee | ||
Comment 3•5 years ago
|
||
Even if we do nothing, we need to know if the value actually changed to see if
we need to propagate it to descendants that explicitly inherit it.
Depends on D31568
Assignee | ||
Comment 4•5 years ago
|
||
Depends on D31569
Assignee | ||
Comment 5•5 years ago
|
||
Can you check that cherry-picking these patches catch the bug?
Reporter | ||
Comment 6•5 years ago
•
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)
Can you check that cherry-picking these patches catch the bug?
Checked the patches based on offset-distance, and run the test. Got something like this:
Unexpected Results
------------------
layout/style/test/test_computed_style_difference.html
FAIL Diffing for offset-distance - Diffing for offset-distance: assert_equals: Didn't handle the inherited change correctly? expected "10px" but got "0px"
Yes, this test (i.e. these patches) catches this bug. :)
Assignee | ||
Comment 7•5 years ago
|
||
Great
Pushed by emilio@crisal.io: https://hg.mozilla.org/integration/mozilla-inbound/rev/18b02f666766 Fix image layer diffing. r=jwatt https://hg.mozilla.org/integration/mozilla-inbound/rev/418d8f9bd4b1 Diff overflow-anchor values. r=jwatt https://hg.mozilla.org/integration/mozilla-inbound/rev/159848724347 Add a test that tests computed style diffing using the property database. r=jwatt
Comment 9•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/18b02f666766
https://hg.mozilla.org/mozilla-central/rev/418d8f9bd4b1
https://hg.mozilla.org/mozilla-central/rev/159848724347
Description
•