[webkit-reviews] review granted: [Bug 18091] font-size doesn't get animated using -webkit-transition : [Attachment 23058] Support font-size animation

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Aug 28 12:45:37 PDT 2008


Darin Adler <darin at apple.com> has granted Dave Hyatt <hyatt at apple.com>'s
request for review:
Bug 18091: font-size doesn't get animated using -webkit-transition
https://bugs.webkit.org/show_bug.cgi?id=18091

Attachment 23058: Support font-size animation
https://bugs.webkit.org/attachment.cgi?id=23058&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
+    // 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


More information about the webkit-reviews mailing list