Bug 20368 - Implement computed style for animation properties
Summary: Implement computed style for animation properties
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-08-12 18:27 PDT by Simon Fraser (smfr)
Modified: 2008-11-20 16:34 PST (History)
1 user (show)

See Also:


Attachments
patch + testcases (20.64 KB, patch)
2008-09-15 17:37 PDT, Dean Jackson
no flags Details | Formatted Diff | Diff
Patch, testcases, changelog (29.12 KB, patch)
2008-11-20 13:48 PST, Simon Fraser (smfr)
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2008-08-12 18:27:57 PDT
getComputedStyle() should work for animation-related properties.
Comment 1 Simon Fraser (smfr) 2008-08-29 15:13:44 PDT
And CSSPropertyWebkitTransformOrigin.
Comment 2 Dean Jackson 2008-09-15 17:37:47 PDT
Created attachment 23452 [details]
patch + testcases

Also fixes computed style for transform origin
Comment 3 Dean Jackson 2008-09-19 17:55:26 PDT
Comment on attachment 23452 [details]
patch + testcases

should not remove the support for transform origin -x and -y
Comment 4 Simon Fraser (smfr) 2008-11-20 13:42:50 PST
I'll take this.
Comment 5 Simon Fraser (smfr) 2008-11-20 13:48:07 PST
Created attachment 25322 [details]
Patch, testcases, changelog
Comment 6 Darin Adler 2008-11-20 14:02:18 PST
Comment on attachment 25322 [details]
Patch, testcases, changelog

> +        case CSSPropertyWebkitAnimationDelay: {
> +            RefPtr<CSSValueList> list = CSSValueList::createCommaSeparated();
> +            const AnimationList* t = style->animations();
> +            if (t) {
> +                for (size_t i = 0; i < t->size(); ++i)
> +                    list->append(CSSPrimitiveValue::create(t->animation(i)->delay(), CSSPrimitiveValue::CSS_S));
> +            } else
> +                list->append(CSSPrimitiveValue::create(RenderStyle::initialAnimationDelay(), CSSPrimitiveValue::CSS_S));
> +            return list.release();
> +        }

These seem like just enough code that it would be better to have them in separate functions rather than in the giant switch statement.

However, that problem would solve itself if we changed this into a table instead of a switch, so I guess it can wait for that.

Where does the name "t" come from?

> +                for (size_t i = 0; i < t->size(); ++i) {
> +                    list->append(CSSPrimitiveValue::create(t->animation(i)->name(), CSSPrimitiveValue::CSS_STRING));
> +                }

Should be no braces here.

> +                    int prop = t->animation(i)->property();

The type of this local variable, and of the property-related functions in Animation, should be CSSPropertyID, not int.

> +                        propertyValue = CSSPrimitiveValue::create(getPropertyName(static_cast<CSSPropertyID>(prop)), CSSPrimitiveValue::CSS_STRING);

Because we don't want typecasts like this one.

r=me
Comment 7 Simon Fraser (smfr) 2008-11-20 16:27:39 PST
r38641 = 541dc7a2b929dcdbb39c15c9c9f683e97f298ab5 (trunk)
	M	WebCore/ChangeLog
	M	WebCore/rendering/style/RenderStyle.h
	M	WebCore/css/CSSComputedStyleDeclaration.cpp
	M	LayoutTests/ChangeLog
	M	LayoutTests/fast/css/computed-style-without-renderer-expected.txt
	M	LayoutTests/fast/css/computed-style-expected.txt
	A	LayoutTests/transforms/computed-style-origin-expected.txt
	A	LayoutTests/transforms/computed-style-origin.html
	M	LayoutTests/svg/css/getComputedStyle-basic-expected.txt
	A	LayoutTests/animations/computed-style.html
	A	LayoutTests/animations/computed-style-expected.txt
r38642 = 1a2c678115f23f608ee19472b79a288d0aabd217 (trunk)

I just noticed that I missed fixing one of Darin's issues. Follow-up coming.
Comment 8 Simon Fraser (smfr) 2008-11-20 16:34:37 PST
I filed bug 22389 on cleaning up animation->property(), since doing so with this change would be more invasive.