Closed Bug 1273807 Opened 5 years ago Closed 4 years ago

mask-position and mask-size animation test case

Categories

(Core :: Layout, defect)

Product:
Core
Component:
Layout
Version:
49 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox49 --- affected
firefox53 --- fixed

People

(Reporter: u459114, Assigned: u459114)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Bug 1273807 - mask-position and mask-size animation test cases.
59 bytes, text/x-review-board-request
heycam
: review+
Details 
We do not have test cases for mask-position and mask-size animation, so there is no way to prevent regression issues such as bug 1273804.
Blocks: mask-image
test_transitions_per_property.html has some tests.  Would they not have caught the regression?
(In reply to David Baron :dbaron: ⌚️UTC-7 from comment #1)
> test_transitions_per_property.html has some tests.  Would they not have
> caught the regression?
Two reasons that I don't think test_transitions_per_property.html can catch this problem
1. we didn't turn on mask-as-shorthand at that moment, so transition test for mask-position is skipped.
2. Transition value is correct, the root cause of this regression is wrong dirty region computing in display item(fixed in bug 1309646), so reftest is needed.
Assignee: aschen → cku
Attachment #8823560 - Flags: review?(cam)
Comment on attachment 8823560 [details]
Bug 1273807 - mask-position and mask-size animation test cases.

https://reviewboard.mozilla.org/r/102094/#review102800

::: layout/reftests/css-animations/mask-anim-ref.html:8
(Diff revision 4)
> +<style>
> +#test {
> +  height: 100px;
> +  width: 100px;
> +  background-repeat: no-repeat;
> +  background-image:url('data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/svg" width="25" height="25"><rect x="0" y="0" width="25" height="25" fill="blue"/></svg>');

Nit: space after ":".  (Same in the other files.)

::: layout/reftests/css-animations/mask-position-after-finish-1b.html:23
(Diff revision 4)
> +  #inner {
> +    height: 100px;
> +    width: 100px;
> +    background-color: blue;
> +    box-sizing: border-box;
> +    /* Apply will-change property to force paint mask on mask layer*/

Nit: space after "layer".  (And in the other files.)

::: layout/reftests/css-animations/mask-size-in-delay-1a.html:4
(Diff revision 4)
> +  @keyframes changeMaskSize {
> +    from { mask-size: 50px 50px; }
> +    to { mask-size: 100px 100px; }
> +  }

Any reason to have the animation (which takes no effect since we're in the delay phase) different from the mask-position tests, where we use the same from and to values?  If not, may as well make them have the same form.

::: layout/reftests/css-animations/mask-size-in-delay-1a.html:19
(Diff revision 4)
> +    mask-position: center center;
> +    mask-image:url('data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/svg" width="25" height="25"><rect x="0" y="0" width="25" height="25" fill="blue"/></svg>');
> +    animation: changeMaskSize 100s 100s infinite;
> +  }
> +</style>
> +<div id="test" class="reftest-wait"></div>

Remove the class="reftest-wait"?
Attachment #8823560 - Flags: review?(cam) → review+
Depends on: 1329091
Pushed by cku@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/75c24febe680
mask-position and mask-size animation test cases. r=heycam
https://hg.mozilla.org/mozilla-central/rev/75c24febe680
Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox53: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.