From ba1ce0580ac16730b373b6fa68801a454f08b575 Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Tue, 13 Aug 2019 22:38:01 +0000 Subject: [PATCH] Bug 1508177 - Use expanded layout viewport height to tell whether we need to render the vertical overlay scrollbar. r=tnikkel In the previous commit, we expanded layout viewport height but during reflow the expanded layout viewport size hasn't reflected in ScrollReflowInput::mContentsOverflowAreas.ScrollableOverflow(). We explicitly need to use the expanded height to tell whether we need vertical vertical scrollbars or not. Note that the expanded layout viewport height should NOT be used for non-overlay scrollbars cases since, for example, if the content width is specified by `width: 100%`, the non-overlay vertical scrollbar narrows down the content's used width a little bit because of the vertical scrollbar width, which in turn the minimum scale gets bigger because the content's width became bit narrower, thus the layout viewport size calculated with the minimum scale gets smaller, then it results the vertical scrollbar no longer needs to be rendered, thus the minimum scale gets back to the original value, then the vertical scroll needs to be rendered again, thus this sequence of processes happens repreatedly. Differential Revision: https://phabricator.services.mozilla.com/D40772 --HG-- extra : moz-landing-system : lando --- layout/generic/nsGfxScrollFrame.cpp | 30 +++++++++++++++++----- layout/generic/nsGfxScrollFrame.h | 2 ++ layout/reftests/meta-viewport/reftest.list | 2 +- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/layout/generic/nsGfxScrollFrame.cpp b/layout/generic/nsGfxScrollFrame.cpp index 9299ab2dc2f3..b95d5a35d3ad 100644 --- a/layout/generic/nsGfxScrollFrame.cpp +++ b/layout/generic/nsGfxScrollFrame.cpp @@ -473,8 +473,24 @@ bool nsHTMLScrollFrame::TryLayout(ScrollReflowInput* aState, std::max(0, compositionSize.height - hScrollbarDesiredHeight)); } - nsRect scrolledRect = mHelper.GetUnsnappedScrolledRectInternal( - aState->mContentsOverflowAreas.ScrollableOverflow(), scrollPortSize); + nsRect overflowRect = aState->mContentsOverflowAreas.ScrollableOverflow(); + // If the content height expanded by the minimum-scale will be taller than + // the scrollable overflow area, we need to expand the area here to tell + // properly whether we need to render the overlay vertical scrollbar. + // NOTE: This expanded size should NOT be used for non-overley scrollbars + // cases since putting the vertical non-overlay scrollbar will make the + // content width narrow a little bit, which in turn the minimum scale value + // becomes a bit bigger than before, then the vertical scrollbar is no longer + // needed, which means the content width becomes the original width, then the + // minimum-scale is changed to the original one, and so forth. + if (mHelper.UsesOverlayScrollbars() && mHelper.mIsUsingMinimumScaleSize && + mHelper.mMinimumScaleSize.height > overflowRect.YMost()) { + MOZ_ASSERT(StaticPrefs::layout_viewport_contains_no_contents_area()); + overflowRect.height += + mHelper.mMinimumScaleSize.height - overflowRect.YMost(); + } + nsRect scrolledRect = + mHelper.GetUnsnappedScrolledRectInternal(overflowRect, scrollPortSize); nscoord oneDevPixel = aState->mBoxState.PresContext()->DevPixelsToAppUnits(1); if (!aForce) { @@ -6152,10 +6168,8 @@ void ScrollFrameHelper::LayoutScrollbars(nsBoxLayoutState& aState, bool hasResizer = HasResizer(); bool scrollbarOnLeft = !IsScrollbarOnRight(); - bool overlayScrollBars = - Document::UseOverlayScrollbars(presShell->GetDocument()); - bool overlayScrollBarsWithZoom = - overlayScrollBars && mIsRoot && presShell->IsVisualViewportSizeSet(); + bool overlayScrollBarsWithZoom = UsesOverlayScrollbars() && mIsRoot && + presShell->IsVisualViewportSizeSet(); nsSize scrollPortClampingSize = mScrollPort.Size(); double res = 1.0; @@ -6985,6 +6999,10 @@ bool ScrollFrameHelper::UsesContainerScrolling() const { return false; } +bool ScrollFrameHelper::UsesOverlayScrollbars() const { + return Document::UseOverlayScrollbars(mOuter->PresShell()->GetDocument()); +} + bool ScrollFrameHelper::DragScroll(WidgetEvent* aEvent) { // Dragging is allowed while within a 20 pixel border. Note that device pixels // are used so that the same margin is used even when zoomed in or out. diff --git a/layout/generic/nsGfxScrollFrame.h b/layout/generic/nsGfxScrollFrame.h index b970ea3c622c..f63d0b2ca832 100644 --- a/layout/generic/nsGfxScrollFrame.h +++ b/layout/generic/nsGfxScrollFrame.h @@ -430,6 +430,8 @@ class ScrollFrameHelper : public nsIReflowCallback { bool UsesContainerScrolling() const; + bool UsesOverlayScrollbars() const; + // In the case where |aDestination| is given, elements which are entirely out // of view when the scroll position is moved to |aDestination| are not going // to be used for snap positions. diff --git a/layout/reftests/meta-viewport/reftest.list b/layout/reftests/meta-viewport/reftest.list index 5b6ce9fa5018..051899629650 100644 --- a/layout/reftests/meta-viewport/reftest.list +++ b/layout/reftests/meta-viewport/reftest.list @@ -20,7 +20,7 @@ pref(layout.viewport_contains_no_contents_area,true) == position-fixed-on-square pref(layout.viewport_contains_no_contents_area,true) fails-if(webrender) == async-scroll-to-no-content-area.html async-scroll-to-no-content-area-ref.html # bug 1571623 to track down the failure on WebRender pref(layout.viewport_contains_no_contents_area,true) == resolution-change-on-landscape-content.html resolution-change-on-landscape-content-ref.html == scrollbars-in-half-height-content.html scrollbars-in-half-height-content-ref.html -pref(layout.viewport_contains_no_contents_area,true) fails-if(usesOverlayScrollbars) == scrollbars-in-landscape-content.html scrollbars-in-landscape-content-ref.html +pref(layout.viewport_contains_no_contents_area,true) == scrollbars-in-landscape-content.html scrollbars-in-landscape-content-ref.html skip-if(!Android) fails-if(geckoview&&webrender) == position-fixed-on-minimum-scale-size.html position-fixed-on-minimum-scale-size-ref.html == position-fixed-out-of-view.html about:blank