[Webkit-unassigned] [Bug 114663] [css3-text] Rendering -webkit-hanging value for text-indent from css3-text

[bugzilla-daemon at webkit.org]

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


Beth Dakin <bdakin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #198235|review?                     |review-
               Flag|                            |




--- Comment #2 from Beth Dakin <bdakin at apple.com>  2013-04-16 20:51:49 PST ---
(From update of attachment 198235)
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.

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