[webkit-reviews] review denied: [Bug 44412] Teach InlineBox about FloatPoint : [Attachment 92637] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jun 1 18:02:32 PDT 2011


Eric Seidel <eric at webkit.org> has denied Levi Weintraub <leviw at chromium.org>'s
request for review:
Bug 44412: Teach InlineBox about FloatPoint
https://bugs.webkit.org/show_bug.cgi?id=44412

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

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=92637&action=review

>> Source/WebCore/rendering/EllipsisBox.cpp:69
>> +	    ty += top() + style->fontMetrics().ascent() - (m_markupBox->y() +
m_markupBox->renderer()->style(m_firstLine)->fontMetrics().ascent());
> 
> Are these going to be confusing for vertical text?  I feel like Darin (or
someone) picked on me about this before.  I'm not sure what the "vertical text
aware" equivalent of "right" would be.

This is my only problem with this patch.  If you can answer if right/left are
OK vs. logicalMaxX or whatever the alternative naming is, than this is fine!

> Source/WebCore/rendering/InlineBox.h:239
> +    float right() const { return left() + logicalWidth(); }

I'm confused how this relates to logicalRight.	Should this use width() instead
of logicalWidth?


More information about the webkit-reviews mailing list