[webkit-reviews] review requested: [Bug 60044] Extract LineInfo class : [Attachment 92117] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue May 3 13:50:37 PDT 2011
Ryosuke Niwa <rniwa at webkit.org> has asked for review:
Bug 60044: Extract LineInfo class
https://bugs.webkit.org/show_bug.cgi?id=60044
Attachment 92117: Patch
https://bugs.webkit.org/attachment.cgi?id=92117&action=review
------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=92117&action=review
> Source/WebCore/rendering/RenderBlock.h:46
> +class LineInfo;
Nit: you should declare this right above LineWidth.
> Source/WebCore/rendering/RenderBlockLineLayout.cpp:69
> + : m_isFirstLine(true)
> + , m_isLastLine(false)
> + , m_isEmpty(true)
> + , m_previousLineBrokeCleanly(true)
I feel like some of these member variables being true by default is arbitrary.
Can we add an enum flags that the constructor takes?
> Source/WebCore/rendering/RenderBlockLineLayout.cpp:1445
> -bool RenderBlock::requiresLineBox(const InlineIterator& it, bool
isLineEmpty, bool previousLineBrokeCleanly)
> +static bool requiresLineBox(const InlineIterator& it, const LineInfo&
lineInfo = LineInfo())
Nice!
> Source/WebCore/rendering/RenderBlockLineLayout.cpp:1458
> + && !skipNonBreakingSpace(it, lineInfo);
Nit: You need one more indentation here.
> Source/WebCore/rendering/RenderBlockLineLayout.cpp:1725
> + bool startingNewParagraph = lineInfo.previousLineBrokeCleanly();
Maybe previousLineBrokeCleanly is a bad name for the member function?
More information about the webkit-reviews
mailing list