[webkit-reviews] review denied: [Bug 23556] On RTL pages, horizontal scrollbar is missing : [Attachment 43559] A workaround fix 3
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Dec 29 04:44:13 PST 2009
Maciej Stachowiak <mjs at apple.com> has denied Hironori Bono
<hbono at chromium.org>'s request for review:
Bug 23556: On RTL pages, horizontal scrollbar is missing
https://bugs.webkit.org/show_bug.cgi?id=23556
Attachment 43559: A workaround fix 3
https://bugs.webkit.org/attachment.cgi?id=43559&action=review
------- Additional Comments from Maciej Stachowiak <mjs at apple.com>
Please try to pare down the comments. It's not necessary to have a comment
(often lengthy) every time int m_rtlMinChildPosition appears. This hurts
readability. Try to limit your comments to saying *why* the code is doing
something, rather than *what* it is doing. "what" should be apparent from the
code. I also agree with Jeremy's comment that the "less than or equal to 0"
condition is best documented with an ASSERT, in which case I expect the
multiple comments explaining that it is less than or equal to 0 are not
necessary.
r- to clean this up. The change otherwise looks ok to me, assuming it matches
what other browsers do, but it may help to have a rendering expert chime in.
More information about the webkit-reviews
mailing list