[webkit-reviews] review denied: [Bug 41535] "vertical-align: middle; " not working on a MathML element : [Attachment 66642] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Sep 6 10:28:01 PDT 2010


mitz at webkit.org has denied François Sausset <sausset at gmail.com>'s request for
review:
Bug 41535: "vertical-align: middle;" not working on a MathML element
https://bugs.webkit.org/show_bug.cgi?id=41535

Attachment 66642: Patch
https://bugs.webkit.org/attachment.cgi?id=66642&action=review

------- Additional Comments from mitz at webkit.org
> +	   if (static_cast<int>(m_xHeight) == 0 && m_ascent > 0)

Why the cast to int? WebKit style is to not write “== 0” and “!= 0”.

> +	       m_xHeight = 0.66 * m_ascent;

Why 0.66? If you used 3 / 2 (or 66 / 100, I don’t know if the difference is
meaningful) you could avoid float arithmetic here. Should this really be based
the on the already-rounded m_ascent, and always rounded down, or would it be
better to use fAscent and round to the nearest integer?

Do other ports besides Mac need a similar changed?


More information about the webkit-reviews mailing list