Bug 18091 - font-size doesn't get animated using -webkit-transition
Summary: font-size doesn't get animated using -webkit-transition
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 525.x (Safari 3.1)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-03-25 16:54 PDT by Benoit Marchant
Modified: 2008-10-12 15:15 PDT (History)
2 users (show)

See Also:


Attachments
example of the bug (4.30 KB, application/xhtml+xml)
2008-03-25 17:03 PDT, Benoit Marchant
no flags Details
Support font-size animation (2.90 KB, patch)
2008-08-28 12:32 PDT, Dave Hyatt
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Benoit Marchant 2008-03-25 16:54:37 PDT
See the attached test case
Comment 1 Benoit Marchant 2008-03-25 17:03:46 PDT
Created attachment 20060 [details]
example of the bug

example of the bug
Comment 2 Dave Hyatt 2008-08-28 12:32:48 PDT
Created attachment 23058 [details]
Support font-size animation
Comment 3 Darin Adler 2008-08-28 12:45:37 PDT
Comment on attachment 23058 [details]
Support font-size animation

+    // FIXME: Use with caution.  Only used for blending font sizes when animating.
+    void setFontSize(int size) {
+        FontDescription desc(fontDescription());
+        desc.setSpecifiedSize(size);
+        desc.setComputedSize(size);
+        setFontDescription(desc);
+        font().update(font().fontSelector());
+    }

Since this function is only used through a function pointer, it's not helpful to inline it. So it would be better if this was in a .cpp file instead of the header.

The comment says "caution" which I think is too vague. And it says FIXME without being clear what should be fixed. It would be stronger if this said something more like:

    // Used for blending font sizes when animating.
    // Would be a bad idea to use this in other cases because <reason header>.

r=me, but please consider my comments and consider if you can add a test case
Comment 4 Dave Hyatt 2008-08-28 12:49:56 PDT
You can really only make manual tests for transitions, since we have no testing framework for them. :(

I'll change the function name to setBlendedFontSize and remove the inlining.
Comment 5 Darin Adler 2008-10-12 15:15:21 PDT
http://trac.webkit.org/changeset/35975