[Webkit-unassigned] [Bug 23556] On RTL pages, horizontal scrollbar is missing

[bugzilla-daemon at webkit.org]

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


Hironori Bono <hbono at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #53150|0                           |1
        is obsolete|                            |
  Attachment #53150|review?                     |
               Flag|                            |
  Attachment #53324|                            |review?
               Flag|                            |




--- Comment #29 from Hironori Bono <hbono at chromium.org>  2010-04-14 02:24:59 PST ---
Created an attachment (id=53324)
 --> (https://bugs.webkit.org/attachment.cgi?id=53324)
A proposed fix 6

Jeremy,

Thank you for your comments.
I have updated my patch to replaced null-checks with ASSERT() expressions and
to use temporary variables.

Regards,

Hironori Bono

(In reply to comment #28)
> (From update of attachment 53150 [details])
> Obligatory disclaimer: I'm not a reviewer, but here are my comments.
> 
> > Index: WebCore/rendering/RenderBox.cpp
> > ===================================================================
> > --- WebCore/rendering/RenderBox.cpp	(revision 57464)
> > +++ WebCore/rendering/RenderBox.cpp	(working copy)
> > @@ -182,9 +182,12 @@ void RenderBox::styleDidChange(StyleDiff
> > +    if (isBody()) {
> >          document()->setTextColor(style()->color());
> > +        if (view() && view()->style())
> 
> Can view() or view()->style() ever be NULL? Same goes for other places in the
> patch.
> 
> >  void RenderBox::updateBoxModelInfoFromStyle()
> > @@ -608,7 +611,11 @@ void RenderBox::paintRootBoxDecorations(
> > +    // The bounding rectangle of an RTL view is (min(leftmostPosition(), 0), 0, docWidth(), docHeight())
> > +    // So, we also include the leftmost position for RTL views so we can fill its entire canvas.
> Great comment.
> 
> > +    if (view() && view()->style()->direction() == RTL && view()->leftmostPosition() < 0)
> Again, are you sure you need to null-check view() ?
> 
> > @@ -117,6 +117,14 @@ void RenderView::layout()
> > +    if (style() && style()->direction() == RTL && leftmostPosition() < 0) {
> Using a temp. variable may ease readability here a bit since you use the same
> expression in the return statement.

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