[Webkit-unassigned] [Bug 113259] [css3-text] Rendering -webkit-each-line value for text-indent from css3-text

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=113259





--- Comment #5 from Levi Weintraub <leviw at chromium.org>  2013-03-27 17:11:48 PST ---
(From update of attachment 194989)
View in context: https://bugs.webkit.org/attachment.cgi?id=194989&action=review

>> Source/WebCore/rendering/RenderBlockLineLayout.cpp:106
>> +        , m_needsIndent(needsIndent)
> 
> I am really not a huge fan of the name |needsIndent| (and again one more boolean arguments). I think shouldIndentText (or equivalent) would be better. Also I would advise to use an enum as we are touching this code anyway

Yessir!

>> Source/WebCore/rendering/RenderBlockLineLayout.cpp:951
>> +static bool requiresIndent(bool firstLine, bool forcedLineBreak, RenderStyle* style)
> 
> Per our coding style, boolean should start with a verb (the style mandates the use of "is" or "did" but really starting with a verb is the intent -> http://www.webkit.org/coding/coding-style.html)
> 
> While we are touching this code, I would prefer if we could fix some of that or at least not add more violations.

+1

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:983
> +    bool forcedLineBreak = lineBox->prevRootBox() && lineBox->prevRootBox()->endsWithBreak();

We usually call this a hardLineBreak (see BidiResolver.h for instance).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.