| Summary: | (Regression) Existing animations are not replaced when filling. | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Dean Jackson <dino> | ||||||||
| Component: | CSS | Assignee: | Dean Jackson <dino> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | simon.fraser | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | All | ||||||||||
| OS: | OS X 10.5 | ||||||||||
| Bug Depends on: | |||||||||||
| Bug Blocks: | 59839 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Dean Jackson
2011-04-21 15:47:33 PDT
Created attachment 90619 [details]
Testcase
This is a ridiculous bug on my part. The filling animation is not postActive, so it isn't being removed. I assumed the animation name change would remove it. Actually, not quite. The animation is being removed in CompositeAnimation, but CA isn't being asked to remove it. Created attachment 91679 [details]
Patch
Comment on attachment 91679 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91679&action=review > LayoutTests/animations/replace-filling-transform.html:2 > +<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" > + "http://www.w3.org/TR/html4/loose.dtd"> Can be just <!DOCTYPE html> > LayoutTests/animations/replace-filling-transform.html:4 > +<html lang="en"> Kill the lang. > LayoutTests/animations/replace-filling-transform.html:7 > + <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> > + <title>Replace a transform animation that is filling</title> Kill, kill > LayoutTests/animations/replace-filling-transform.html:8 > + <style type="text/css" media="screen"> Kill the attributes > LayoutTests/animations/replace-filling-transform.html:23 > + from { > + -webkit-transform: translate3d(100px, 0, 0); > + } > + to { > + -webkit-transform: translate3d(200px, 0, 0); > + } If this test is about hardware animation, then it should go into animation/3d > LayoutTests/animations/replace-filling-transform.html:35 > + <script type="text/javascript" charset="utf-8"> Kill the attributes > LayoutTests/animations/replace-filling-transform.html:49 > + if (computedValue == expectedValue) { > + result.innerHTML = "PASS - final state was " + expectedValue; > + } else { > + result.innerHTML = "FAIL - final state was " + computedValue + " expected " + expectedValue; > + } No need for braces on single lines. > LayoutTests/animations/replace-filling-transform.html:53 > + } Ditto. > LayoutTests/animations/replace-filling-transform.html:71 > + if (window.layoutTestController) { > + layoutTestController.dumpAsText(); > + layoutTestController.waitUntilDone(); > + } We normally put this code bare in the <script> tag. Comment on attachment 91679 [details]
Patch
The test needs to dump pixels to reveal the failure. You can use dumpAsText(true) to dump pixels and text results.
Created attachment 91698 [details]
Patch
updated from review comments and added pixel test (which means the test will fail where it should fail - even when getComputedStyle lies) Comment on attachment 91698 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91698&action=review r=me but please consider improving the pixel test. > LayoutTests/animations/3d/replace-filling-transform.html:78 > +This sets up two animations. It runs the first and then triggers the 2nd. The first fills > +forwards, but should still be replaced by the second. The first is a horizontal animation, the second is > +vertical, so the box should end up translated vertically down the page. Please put this text into an HTML comment, so that it doesn't show in the pixel result. That way, the result can be cross-platform. Also, make #result have opacity: 0 so it doesn't pollute the pixel result. For extra points, make the test box a 100x100px green square, and put a red square under its correct end position. Then when the test fails, red will be visible. If you've done all this, the pixel result can then be platform-agnostic and go into LayoutTests/animations/3d Great suggestions. Done in http://trac.webkit.org/changeset/85338 |