[Webkit-unassigned] [Bug 113259] [css3-text] Rendering -webkit-each-line value for text-indent from css3-text
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Mar 27 20:42:14 PDT 2013
https://bugs.webkit.org/show_bug.cgi?id=113259
--- Comment #6 from Jaehun Lim <ljaehun.lim at samsung.com> 2013-03-27 20:40:23 PST ---
(From update of attachment 194989)
View in context: https://bugs.webkit.org/attachment.cgi?id=194989&action=review
Thank you for your review, Julien Chaffraix and Levi Weintraub.
>>> 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!
Add enum IndentTextOrNot and use bool shouldIndentText.
>>> 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
Use m_shouldIndentText and shouldIndentText().
>> Source/WebCore/rendering/RenderBlockLineLayout.cpp:983
>> + bool forcedLineBreak = lineBox->prevRootBox() && lineBox->prevRootBox()->endsWithBreak();
>
> We usually call this a hardLineBreak (see BidiResolver.h for instance).
Use isHardLineBreak.
>> LayoutTests/platform/win/TestExpectations:2492
>> +fast/css3-text/css3-text-indent/each-line.html
>
> Why only windows and not mac / chromium / qt / gtk? Can we just skip the whole css3-text-indent directory?
Yes, other ports except windows skip the whole directory, fast/css3-text/css3-text-indent/.
I change this line to skip 'css3-text-indent' directory as other ports.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list