[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
----------------------------------------------------------------------------
            Summary|rtl horizontal scrollbar /  |REGRESSION: rtl horizontal
                   |resize bug - Body shifts on |scrollbar / resize bug -
                   |resize when scrolled all    |Body shifts on resize when
                   |the way to the left         |scrolled all the way to the
                   |                            |left
             Status|UNCONFIRMED                 |NEW
                 CC|                            |xji at chromium.org
     Ever Confirmed|0                           |1




--- Comment #2 from Xiaomei Ji <xji at chromium.org>  2011-10-19 16:17:13 PST ---
It appears regressed in http://trac.webkit.org/changeset/76395/trunk.

If I remove the condition checking in ScrollAnimator::scrollToOffsetWithoutAnimation(), the test case works ok (tested in chromium, Safari in Mac works fine since it has its own ScrollAnimatorMac.mm, in which, ScollTo() is always called without condition).

But I am not sure removing the condition checking is the right fix.
Would appreciate comments from mitz (who added ScrollView.cpp#L586) and/or weinig (who is more familiar with ScrollAnimation).

The reason of the failure is that:
when horizontal scrollbar is at leftmost, ScrollAnimator::m_currentPosX/Y == 0.

when enlarge browser size, following is part of the call stack:
        ......
     ScrollView::updateScrollbars(...)
     ScrollView::setScrollOrigin(...)
     FrameView::adjustViewSize(...)
        .....

Assume m_scrollOrigin.x was 200 previously, and is 150 now, at inline http://trac.webkit.org/browser/trunk/Source/WebCore/platform/ScrollView.cpp#L586
desiredOffset is the old scroll offset, which is -200, and adjustScrollPositionWithinRange() will adjust it within the range of (-150, 0) using current m_scrollOrigin, and scrollPoint is always >= 0.

After the above line, the scrollPoint passed to ScrollAnimator::scrollToOffsetWithoutAnimation() is (0, 0), since it is the same as m_currentPosX/Y, notifyPositionChanged() will *not* be called, so FrameView::scrollTo() using current m_scrollOrigin etc. are not called, caused wrong/mis-match of scroll position with document body.

It looks to me that ScrollAnimator::m_currentPosX and the parameter passing to ScrollAnimator::scrollToOffsetWithoutAnimation() are positive values even the scroll positions are negative values for RTL pages. I do not know the reason, but if that is the case, ScrollView.cpp#L58 seems doing the right thing, and we probably have to always scroll without condition checking.

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