[webkit-reviews] review denied: [Bug 114663] [css3-text] Rendering -webkit-hanging value for text-indent from css3-text : [Attachment 198235] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Apr 16 20:53:31 PDT 2013
Beth Dakin <bdakin at apple.com> has denied Jaehun Lim <ljaehun.lim at samsung.com>'s
request for review:
Bug 114663: [css3-text] Rendering -webkit-hanging value for text-indent from
css3-text
https://bugs.webkit.org/show_bug.cgi?id=114663
Attachment 198235: Patch
https://bugs.webkit.org/attachment.cgi?id=198235&action=review
------- Additional Comments from Beth Dakin <bdakin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=198235&action=review
Looks pretty close! I just have a few comments that I would like you to
address. Let's do another round on this.
> Source/WebCore/ChangeLog:9
> + "hanging" means "Inverts which lines are affected."
Maybe include a link to the spec?
> Source/WebCore/rendering/RenderBlockLineLayout.cpp:1000
> + shouldIndentText == IndentText ? shouldIndentText = DoNotIndentText
: shouldIndentText = IndentText;
I find this line very difficult to read. I think it is more common in WebKit to
write this with the assignment operator always on the left like so:
shouldIndentText = shouldIndentText == IndentText ? DoNotIndentText :
IndentText;
> LayoutTests/ChangeLog:11
> + *
fast/css3-text/css3-text-indent/text-indent-each-line-expected.html: Removed.
Why is this file being removed? Seem like maybe removing it was a mistake? If
it was not a mistake, you should explain it in the Changelog.
More information about the webkit-reviews
mailing list