Closed Bug 1552344 Opened 8 months ago Closed 8 months ago

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)

enhancement

Tracking

()

RESOLVED FIXED
mozilla68
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>
Flags: needinfo?(emilio)

Note: The current try doesn't catch this if someone or I forget to add this change hint.

Assignee: nobody → emilio
Flags: needinfo?(emilio)

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.

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

Can you check that cherry-picking these patches catch the bug?

Flags: needinfo?(boris.chiou)

(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. :)

Flags: needinfo?(boris.chiou)

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
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.