[Webkit-unassigned] [Bug 70395] REGRESSION: rtl horizontal scrollbar / resize bug - Body shifts on resize when scrolled all the way to the left

[bugzilla-daemon at webkit.org]

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


Xiaomei Ji <xji at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #113261|0                           |1
        is obsolete|                            |
 Attachment #113261|review?                     |
               Flag|                            |
 Attachment #113335|                            |review?
               Flag|                            |




--- Comment #19 from Xiaomei Ji <xji at chromium.org>  2011-11-02 11:00:19 PST ---
Created an attachment (id=113335)
 --> (https://bugs.webkit.org/attachment.cgi?id=113335&action=review)
patch w/ layout test

updated per comment #18. Thanks for the quick drive-by review.



(In reply to comment #18)
> (From update of attachment 113261 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=113261&action=review
> 
> Just a drive by review.  I don't really understand how the various Scroll* classes fit together.
> 
> > Source/WebCore/platform/ScrollAnimator.cpp:82
> >          m_currentPosX = offset.x();
> >          m_currentPosY = offset.y();
> >          notifyPositionChanged();
> 
> Nit: unindent

done.

> 
> > Source/WebCore/platform/ScrollableArea.cpp:73
> > +    } else
> > +        m_scrollOriginChanged = false;
> 
> Is this else necessary?  If so, why don't we set m_scrollOriginChanged = false in setScrollOrigin(X|Y)?

That is a good question.
Look at it again, I think the same as m_scrollOriginChanged is only accessed in ScrollView::updateScrollbars(), its value should be only reset to false in ScrollView::updateScrollbars(), so that ScrollView::updateScrollbars() is able to catch the once changed m_scrollOrigin and set ScrollAnimator::m_currentPosX|Y correctly.

As to the reason why not set m_scrollOriginChanged = false in setScrollOrigin(X|Y) is that: if you setScrollOriginX with a different scrollOriginX value, but then setScrollOriginY with the same scrollOriginY value, resetting m_scrollOriginChanged = false in setScrollOriginY will override the m_scrollOriginChanged = true value set in setScrollOriginX.

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