Skip to content

Commit

Permalink
Bug 1644638. ScrollFrameHelper::ReflowFinished should use GetVisualSc…
Browse files Browse the repository at this point in the history
…rollRange instead of trying to calculate it itself. r=kats

This avoids a bug where the scroll range can be negative.

Differential Revision: https://phabricator.services.mozilla.com/D79029
  • Loading branch information
tnikkel committed Jun 12, 2020
1 parent 5672e28 commit f8c6af6
Showing 1 changed file with 16 additions and 21 deletions.
37 changes: 16 additions & 21 deletions layout/generic/nsGfxScrollFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6024,13 +6024,6 @@ bool ScrollFrameHelper::ReflowFinished() {
}
}

nsRect scrolledContentRect = GetScrolledRect();
nsSize scrollClampingScrollPort = GetVisualViewportSize();
nscoord minX = scrolledContentRect.x;
nscoord maxX = scrolledContentRect.XMost() - scrollClampingScrollPort.width;
nscoord minY = scrolledContentRect.y;
nscoord maxY = scrolledContentRect.YMost() - scrollClampingScrollPort.height;

// Suppress handling of the curpos attribute changes we make here.
NS_ASSERTION(!mFrameIsUpdatingScrollbar, "We shouldn't be reentering here");
mFrameIsUpdatingScrollbar = true;
Expand All @@ -6045,6 +6038,9 @@ bool ScrollFrameHelper::ReflowFinished() {
// for scrollbars. XXXmats is this still true now that we have a script
// blocker in this scope? (if not, remove the weak frame checks below).
if (vScroll || hScroll) {
nsSize visualViewportSize = GetVisualViewportSize();
nsRect scrollRange = GetVisualScrollRange();

AutoWeakFrame weakFrame(mOuter);
nsPoint scrollPos = GetScrollPosition();
nsSize lineScrollAmount = GetLineScrollAmount();
Expand All @@ -6053,29 +6049,28 @@ bool ScrollFrameHelper::ReflowFinished() {
Preferences::GetInt("toolkit.scrollbox.verticalScrollDistance",
NS_DEFAULT_VERTICAL_SCROLL_DISTANCE);
nscoord increment = lineScrollAmount.height * kScrollMultiplier;
// We normally use (scrollArea.height - increment) for height
// of page scrolling. However, it is too small when
// increment is very large. (If increment is larger than
// scrollArea.height, direction of scrolling will be opposite).
// To avoid it, we use (float(scrollArea.height) * 0.8) as
// lower bound value of height of page scrolling. (bug 383267)
// We normally use (visualViewportSize.height - increment) for height of
// page scrolling. However, it is too small when increment is very large.
// (If increment is larger than visualViewportSize.height, direction of
// scrolling will be opposite). To avoid it, we use
// (float(visualViewportSize.height) * 0.8) as lower bound value of height
// of page scrolling. (bug 383267)
// XXX shouldn't we use GetPageScrollAmount here?
nscoord pageincrement =
nscoord(scrollClampingScrollPort.height - increment);
nscoord pageincrement = nscoord(visualViewportSize.height - increment);
nscoord pageincrementMin =
nscoord(float(scrollClampingScrollPort.height) * 0.8);
FinishReflowForScrollbar(vScroll, minY, maxY, scrollPos.y,
std::max(pageincrement, pageincrementMin),
increment);
nscoord(float(visualViewportSize.height) * 0.8);
FinishReflowForScrollbar(
vScroll, scrollRange.y, scrollRange.YMost(), scrollPos.y,
std::max(pageincrement, pageincrementMin), increment);
}
if (hScroll) {
const double kScrollMultiplier =
Preferences::GetInt("toolkit.scrollbox.horizontalScrollDistance",
NS_DEFAULT_HORIZONTAL_SCROLL_DISTANCE);
nscoord increment = lineScrollAmount.width * kScrollMultiplier;
FinishReflowForScrollbar(
hScroll, minX, maxX, scrollPos.x,
nscoord(float(scrollClampingScrollPort.width) * 0.8), increment);
hScroll, scrollRange.x, scrollRange.XMost(), scrollPos.x,
nscoord(float(visualViewportSize.width) * 0.8), increment);
}
NS_ENSURE_TRUE(weakFrame.IsAlive(), false);
}
Expand Down

0 comments on commit f8c6af6

Please sign in to comment.