diff --git a/mozilla/layout/generic/nsGfxScrollFrame.cpp b/mozilla/layout/generic/nsGfxScrollFrame.cpp index 67d9c1a8e8e..a9359e3b359 100644 --- a/mozilla/layout/generic/nsGfxScrollFrame.cpp +++ b/mozilla/layout/generic/nsGfxScrollFrame.cpp @@ -151,6 +151,10 @@ public: nsresult Layout(nsBoxLayoutState& aState); nsresult LayoutBox(nsBoxLayoutState& aState, nsIBox* aBox, const nsRect& aRect); + + // Like ScrollPositionDidChange, but initiated by this frame rather than from the + // scrolling view + void InternalScrollPositionDidChange(nscoord aX, nscoord aY); PRBool AddRemoveScrollbar (PRBool& aHasScrollbar, nscoord& aXY, @@ -199,7 +203,8 @@ public: PRPackedBool mFirstPass; PRPackedBool mIsRoot; PRPackedBool mNeverReflowed; - PRPackedBool mScrollingInitiated; + PRPackedBool mViewInitiatedScroll; + PRPackedBool mFrameInitiatedScroll; }; NS_IMPL_ISUPPORTS2(nsGfxScrollFrameInner, nsIDocumentObserver, nsIScrollPositionListener) @@ -227,7 +232,8 @@ nsGfxScrollFrame::nsGfxScrollFrame(nsIPresShell* aShell, nsIDocument* aDocument, mPresContext = nsnull; mInner->mIsRoot = PR_FALSE; mInner->mNeverReflowed = PR_TRUE; - mInner->mScrollingInitiated = PR_FALSE; + mInner->mViewInitiatedScroll = PR_FALSE; + mInner->mFrameInitiatedScroll = PR_FALSE; SetLayoutManager(nsnull); } @@ -913,6 +919,19 @@ nsGfxScrollFrameInner::ScrollPositionWillChange(nsIScrollableView* aScrollable, return NS_OK; } +/** + * Called when someone (external or this frame) moves the scroll area. + */ +void +nsGfxScrollFrameInner::InternalScrollPositionDidChange(nscoord aX, nscoord aY) +{ + if (mVScrollbarBox) + SetAttribute(mVScrollbarBox, nsXULAtoms::curpos, aY); + + if (mHScrollbarBox) + SetAttribute(mHScrollbarBox, nsXULAtoms::curpos, aX); +} + /** * Called if something externally moves the scroll area * This can happen if the user pages up down or uses arrow keys @@ -921,13 +940,15 @@ nsGfxScrollFrameInner::ScrollPositionWillChange(nsIScrollableView* aScrollable, NS_IMETHODIMP nsGfxScrollFrameInner::ScrollPositionDidChange(nsIScrollableView* aScrollable, nscoord aX, nscoord aY) { - if (mVScrollbarBox) - SetAttribute(mVScrollbarBox, nsXULAtoms::curpos, aY); - - if (mHScrollbarBox) - SetAttribute(mHScrollbarBox, nsXULAtoms::curpos, aX); - - return NS_OK; + NS_ASSERTION(!mViewInitiatedScroll, "Cannot reenter ScrollPositionDidChange"); + + mViewInitiatedScroll = PR_TRUE; + + InternalScrollPositionDidChange(aX, aY); + + mViewInitiatedScroll = PR_FALSE; + + return NS_OK; } NS_IMETHODIMP @@ -938,11 +959,30 @@ nsGfxScrollFrameInner::AttributeChanged(nsIDocument *aDocument, PRInt32 aModType, nsChangeHint aHint) { - // Don't reenter if we're getting this attribute change notification - // because we caused the view to scroll and the view is telling - // us about it. - if (mScrollingInitiated) return NS_OK; - + // Attribute changes on the scrollbars happen in one of three ways: + // 1) The scrollbar changed the attribute in response to some user event + // 2) We changed the attribute in response to a ScrollPositionDidChange + // callback from the scrolling view + // 3) We changed the attribute to adjust the scrollbars for the start + // of a smooth scroll operation + // + // In case 2), we don't need to scroll the view because the scrolling + // has already happened. In case 3) we don't need to scroll because + // we're just adjusting the scrollbars back to the correct setting + // for the view. + // + // We used to detect this case implicitly because we'd compare the + // scrollbar attributes with the view's current scroll position and + // bail out if they were equal. But that approach is fragile; it can + // fail when, for example, the view scrolls horizontally and + // vertically simultaneously; we'll get here when only the vertical + // attribute has been set, so the attributes and the view scroll + // position don't yet agree, and we'd tell the view to scroll to the + // new vertical position and the old horizontal position! Even worse + // things could happen when smooth scrolling got involved ... crashes + // and other terrors. + if (mViewInitiatedScroll || mFrameInitiatedScroll) return NS_OK; + if (mHScrollbarBox && mVScrollbarBox) { nsIFrame* hframe = nsnull; @@ -1000,19 +1040,20 @@ nsGfxScrollFrameInner::AttributeChanged(nsIDocument *aDocument, PRBool isSmooth = vcontent->HasAttr(kNameSpaceID_None, nsXULAtoms::smooth) || hcontent->HasAttr(kNameSpaceID_None, nsXULAtoms::smooth); - // Remember that we asked the view to scroll, so we don't need to - // take notice of any attribute change caused by the view's scrolling - // action. - mScrollingInitiated = PR_TRUE; - ScrollbarChanged(mOuter->mPresContext, x*mOnePixel, y*mOnePixel, isSmooth ? NS_VMREFRESH_SMOOTHSCROLL : 0); if (isSmooth) { // Make sure an attribute-setting callback occurs even if the view didn't actually move yet // We need to make sure other listeners see that the scroll position is not (yet) // what they thought it was. s->GetScrollPosition(curPosX, curPosY); - ScrollPositionDidChange(s, curPosX, curPosY); + + NS_ASSERTION(!mFrameInitiatedScroll, "Unexpected reentry"); + // Make sure we don't do anything in when the view calls us back for this + // scroll operation. + mFrameInitiatedScroll = PR_TRUE; + InternalScrollPositionDidChange(curPosX, curPosY); + mFrameInitiatedScroll = PR_FALSE; } - mScrollingInitiated = PR_FALSE; + ScrollbarChanged(mOuter->mPresContext, x*mOnePixel, y*mOnePixel, isSmooth ? NS_VMREFRESH_SMOOTHSCROLL : 0); // Fire the onScroll event now that we have scrolled nsCOMPtr presShell; diff --git a/mozilla/layout/html/base/src/nsGfxScrollFrame.cpp b/mozilla/layout/html/base/src/nsGfxScrollFrame.cpp index 67d9c1a8e8e..a9359e3b359 100644 --- a/mozilla/layout/html/base/src/nsGfxScrollFrame.cpp +++ b/mozilla/layout/html/base/src/nsGfxScrollFrame.cpp @@ -151,6 +151,10 @@ public: nsresult Layout(nsBoxLayoutState& aState); nsresult LayoutBox(nsBoxLayoutState& aState, nsIBox* aBox, const nsRect& aRect); + + // Like ScrollPositionDidChange, but initiated by this frame rather than from the + // scrolling view + void InternalScrollPositionDidChange(nscoord aX, nscoord aY); PRBool AddRemoveScrollbar (PRBool& aHasScrollbar, nscoord& aXY, @@ -199,7 +203,8 @@ public: PRPackedBool mFirstPass; PRPackedBool mIsRoot; PRPackedBool mNeverReflowed; - PRPackedBool mScrollingInitiated; + PRPackedBool mViewInitiatedScroll; + PRPackedBool mFrameInitiatedScroll; }; NS_IMPL_ISUPPORTS2(nsGfxScrollFrameInner, nsIDocumentObserver, nsIScrollPositionListener) @@ -227,7 +232,8 @@ nsGfxScrollFrame::nsGfxScrollFrame(nsIPresShell* aShell, nsIDocument* aDocument, mPresContext = nsnull; mInner->mIsRoot = PR_FALSE; mInner->mNeverReflowed = PR_TRUE; - mInner->mScrollingInitiated = PR_FALSE; + mInner->mViewInitiatedScroll = PR_FALSE; + mInner->mFrameInitiatedScroll = PR_FALSE; SetLayoutManager(nsnull); } @@ -913,6 +919,19 @@ nsGfxScrollFrameInner::ScrollPositionWillChange(nsIScrollableView* aScrollable, return NS_OK; } +/** + * Called when someone (external or this frame) moves the scroll area. + */ +void +nsGfxScrollFrameInner::InternalScrollPositionDidChange(nscoord aX, nscoord aY) +{ + if (mVScrollbarBox) + SetAttribute(mVScrollbarBox, nsXULAtoms::curpos, aY); + + if (mHScrollbarBox) + SetAttribute(mHScrollbarBox, nsXULAtoms::curpos, aX); +} + /** * Called if something externally moves the scroll area * This can happen if the user pages up down or uses arrow keys @@ -921,13 +940,15 @@ nsGfxScrollFrameInner::ScrollPositionWillChange(nsIScrollableView* aScrollable, NS_IMETHODIMP nsGfxScrollFrameInner::ScrollPositionDidChange(nsIScrollableView* aScrollable, nscoord aX, nscoord aY) { - if (mVScrollbarBox) - SetAttribute(mVScrollbarBox, nsXULAtoms::curpos, aY); - - if (mHScrollbarBox) - SetAttribute(mHScrollbarBox, nsXULAtoms::curpos, aX); - - return NS_OK; + NS_ASSERTION(!mViewInitiatedScroll, "Cannot reenter ScrollPositionDidChange"); + + mViewInitiatedScroll = PR_TRUE; + + InternalScrollPositionDidChange(aX, aY); + + mViewInitiatedScroll = PR_FALSE; + + return NS_OK; } NS_IMETHODIMP @@ -938,11 +959,30 @@ nsGfxScrollFrameInner::AttributeChanged(nsIDocument *aDocument, PRInt32 aModType, nsChangeHint aHint) { - // Don't reenter if we're getting this attribute change notification - // because we caused the view to scroll and the view is telling - // us about it. - if (mScrollingInitiated) return NS_OK; - + // Attribute changes on the scrollbars happen in one of three ways: + // 1) The scrollbar changed the attribute in response to some user event + // 2) We changed the attribute in response to a ScrollPositionDidChange + // callback from the scrolling view + // 3) We changed the attribute to adjust the scrollbars for the start + // of a smooth scroll operation + // + // In case 2), we don't need to scroll the view because the scrolling + // has already happened. In case 3) we don't need to scroll because + // we're just adjusting the scrollbars back to the correct setting + // for the view. + // + // We used to detect this case implicitly because we'd compare the + // scrollbar attributes with the view's current scroll position and + // bail out if they were equal. But that approach is fragile; it can + // fail when, for example, the view scrolls horizontally and + // vertically simultaneously; we'll get here when only the vertical + // attribute has been set, so the attributes and the view scroll + // position don't yet agree, and we'd tell the view to scroll to the + // new vertical position and the old horizontal position! Even worse + // things could happen when smooth scrolling got involved ... crashes + // and other terrors. + if (mViewInitiatedScroll || mFrameInitiatedScroll) return NS_OK; + if (mHScrollbarBox && mVScrollbarBox) { nsIFrame* hframe = nsnull; @@ -1000,19 +1040,20 @@ nsGfxScrollFrameInner::AttributeChanged(nsIDocument *aDocument, PRBool isSmooth = vcontent->HasAttr(kNameSpaceID_None, nsXULAtoms::smooth) || hcontent->HasAttr(kNameSpaceID_None, nsXULAtoms::smooth); - // Remember that we asked the view to scroll, so we don't need to - // take notice of any attribute change caused by the view's scrolling - // action. - mScrollingInitiated = PR_TRUE; - ScrollbarChanged(mOuter->mPresContext, x*mOnePixel, y*mOnePixel, isSmooth ? NS_VMREFRESH_SMOOTHSCROLL : 0); if (isSmooth) { // Make sure an attribute-setting callback occurs even if the view didn't actually move yet // We need to make sure other listeners see that the scroll position is not (yet) // what they thought it was. s->GetScrollPosition(curPosX, curPosY); - ScrollPositionDidChange(s, curPosX, curPosY); + + NS_ASSERTION(!mFrameInitiatedScroll, "Unexpected reentry"); + // Make sure we don't do anything in when the view calls us back for this + // scroll operation. + mFrameInitiatedScroll = PR_TRUE; + InternalScrollPositionDidChange(curPosX, curPosY); + mFrameInitiatedScroll = PR_FALSE; } - mScrollingInitiated = PR_FALSE; + ScrollbarChanged(mOuter->mPresContext, x*mOnePixel, y*mOnePixel, isSmooth ? NS_VMREFRESH_SMOOTHSCROLL : 0); // Fire the onScroll event now that we have scrolled nsCOMPtr presShell; diff --git a/mozilla/view/src/nsScrollPortView.cpp b/mozilla/view/src/nsScrollPortView.cpp index adb62d9d13a..5aaf1985a97 100644 --- a/mozilla/view/src/nsScrollPortView.cpp +++ b/mozilla/view/src/nsScrollPortView.cpp @@ -222,69 +222,23 @@ NS_IMETHODIMP nsScrollPortView::GetScrollPreference(nsScrollPreference &aScrollP static void ComputeVelocities(PRInt32 aCurVelocity, nscoord aCurPos, nscoord aDstPos, PRInt32* aVelocities) { - PRInt32 absCurVelocity; - PRInt32 direction; - - if (aCurPos < aDstPos) { - direction = 1; - if (aCurVelocity < 0) { - absCurVelocity = 0; - } else { - absCurVelocity = aCurVelocity; - } - } else { - direction = -1; - if (aCurVelocity > 0) { - absCurVelocity = 0; - } else { - absCurVelocity = -aCurVelocity; - } - } - - PRInt32 absDistance = direction*(aDstPos - aCurPos); - - // The target velocity is the velocity we want to reach at frame N/2. We choose the - // target velocity so that after coming to a stop at frame N, we will have reached - // the scroll destination (actually we'll overshoot because of rounding, but that's - // the idea). - // It's chosen to satisfy - // aDstPos - aCurPos = N/2 (aTargetVelocity/2) + N/2 ((aCurVelocity + aTargetVelocity)/2) - // distance travelled avg. velocity in 1st half avg. velocity in 2nd half - PRInt32 absTargetVelocity = (PRInt32)ceil(2.0*absDistance/SMOOTH_SCROLL_FRAMES - 0.5*absCurVelocity); - - // if we're going too fast already, slow down to the target velocity - if (absCurVelocity > absTargetVelocity) { - absCurVelocity = absTargetVelocity; - } - PRInt32 i; - const int halfFrames = SMOOTH_SCROLL_FRAMES/2; - for (i = 0; i < halfFrames; i++) { - aVelocities[i*2] = PR_MAX(0, absCurVelocity + (absTargetVelocity*i + halfFrames - 1)/halfFrames); - } - for (i = halfFrames; i < halfFrames*2; i++) { - aVelocities[i*2] = PR_MAX(0, (absTargetVelocity*(halfFrames*2 - (i - 1)) + halfFrames - 1)/halfFrames); - } + PRInt32 direction = (aCurPos < aDstPos ? 1 : -1); + PRInt32 absDelta = (aDstPos - aCurPos)*direction; + PRInt32 baseVelocity = absDelta/SMOOTH_SCROLL_FRAMES; - PRInt32 total = 0; for (i = 0; i < SMOOTH_SCROLL_FRAMES; i++) { - total += aVelocities[i*2]; + aVelocities[i*2] = baseVelocity; } - PRInt32 leftover = total - absDistance; - NS_ASSERTION(leftover >= 0, "Lost some distance due to rounding? We should have been rounding up only"); - - while (leftover > 0) { - for (i = SMOOTH_SCROLL_FRAMES - 1; i >= 0; i--) { - if (aVelocities[i*2] > 0) { - aVelocities[i*2]--; - leftover--; - if (leftover <= 0) { - break; - } - } + nscoord total = baseVelocity*SMOOTH_SCROLL_FRAMES; + for (i = 0; i < SMOOTH_SCROLL_FRAMES; i++) { + if (total < absDelta) { + aVelocities[i*2]++; + total++; } } - + NS_ASSERTION(total == absDelta, "Invalid velocity sum"); + for (i = 0; i < SMOOTH_SCROLL_FRAMES; i++) { aVelocities[i*2] *= direction; } @@ -392,10 +346,6 @@ NS_IMETHODIMP nsScrollPortView::ScrollTo(nscoord aDestinationX, nscoord aDestina ComputeVelocities(currentVelocityY, mOffsetY, mSmoothScroll->mDestinationY, mSmoothScroll->mVelocities + 1); - // call a scroll immediately, don't wait for the timer to callback. - // this improves responsiveness - IncrementalScroll(); - return NS_OK; }