From f9e7ddcde45174f097e59b09adef5b77a127a026 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 27 Mar 2023 20:54:53 +0000 Subject: [PATCH] Bug 1824236 - Stop using XUL layout for scrollbars. r=jwatt This rewrites scrollbar layout to work with regular reflow rather than box layout. Overall it's about the same amount of code (mostly because nsScrollbarFrame::Reflow is sorta hand-rolled), but it cleans up a bit and it is progress towards removing XUL layout altogether, without getting into much deeper refactoring. This also blocks some other performance improvements and refactorings I want to make in this code. We make some assumptions to simplify the code that to some extent were made already before, both explicitly and by virtue of using XUL layout. In particular, we assume that scrollbar / slider / thumb has no border or padding and that the writing-mode is horizontal ltr. Differential Revision: https://phabricator.services.mozilla.com/D173489 --- .../test/pointerevents/test_bug1697769.xhtml | 2 +- .../mochitest/helper_overflowhidden_zoom.html | 2 +- layout/base/nsCSSFrameConstructor.cpp | 9 +- layout/generic/FrameClasses.py | 3 +- layout/generic/ScrollbarActivity.cpp | 1 + layout/generic/crashtests/369038-1.xhtml | 29 - layout/generic/crashtests/crashtests.list | 1 - layout/generic/nsGfxScrollFrame.cpp | 238 +++--- layout/generic/nsGfxScrollFrame.h | 11 +- layout/generic/nsInlineFrame.cpp | 2 +- layout/style/res/scrollbars.css | 46 +- ...est_viewport_scrollbar_causing_reflow.html | 21 +- layout/xul/SimpleXULLeafFrame.cpp | 37 + layout/xul/SimpleXULLeafFrame.h | 46 ++ layout/xul/moz.build | 1 + layout/xul/nsBoxFrame.cpp | 31 - layout/xul/nsBoxFrame.h | 8 - layout/xul/nsIScrollbarMediator.h | 2 +- layout/xul/nsScrollbarButtonFrame.cpp | 5 +- layout/xul/nsScrollbarButtonFrame.h | 13 +- layout/xul/nsScrollbarFrame.cpp | 173 +++-- layout/xul/nsScrollbarFrame.h | 52 +- layout/xul/nsSliderFrame.cpp | 705 ++++++++---------- layout/xul/nsSliderFrame.h | 55 +- layout/xul/test/windowminmaxsize1.xhtml | 2 +- layout/xul/test/windowminmaxsize10.xhtml | 2 +- layout/xul/test/windowminmaxsize2.xhtml | 2 +- layout/xul/test/windowminmaxsize3.xhtml | 2 +- layout/xul/test/windowminmaxsize4.xhtml | 2 +- layout/xul/test/windowminmaxsize5.xhtml | 2 +- layout/xul/test/windowminmaxsize6.xhtml | 2 +- layout/xul/test/windowminmaxsize7.xhtml | 2 +- layout/xul/test/windowminmaxsize8.xhtml | 2 +- layout/xul/test/windowminmaxsize9.xhtml | 2 +- layout/xul/tree/nsTreeBodyFrame.cpp | 16 +- layout/xul/tree/nsTreeBodyFrame.h | 2 +- .../tests/chrome/test_bug1048178.xhtml | 15 +- .../content/tests/chrome/test_scrollbar.xhtml | 13 +- 38 files changed, 773 insertions(+), 786 deletions(-) delete mode 100644 layout/generic/crashtests/369038-1.xhtml create mode 100644 layout/xul/SimpleXULLeafFrame.cpp create mode 100644 layout/xul/SimpleXULLeafFrame.h diff --git a/dom/events/test/pointerevents/test_bug1697769.xhtml b/dom/events/test/pointerevents/test_bug1697769.xhtml index 073120db9bbe..95455d9be109 100644 --- a/dom/events/test/pointerevents/test_bug1697769.xhtml +++ b/dom/events/test/pointerevents/test_bug1697769.xhtml @@ -12,7 +12,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1697769 - + diff --git a/gfx/layers/apz/test/mochitest/helper_overflowhidden_zoom.html b/gfx/layers/apz/test/mochitest/helper_overflowhidden_zoom.html index 2f676c54b0ae..6c12008e4baf 100644 --- a/gfx/layers/apz/test/mochitest/helper_overflowhidden_zoom.html +++ b/gfx/layers/apz/test/mochitest/helper_overflowhidden_zoom.html @@ -15,7 +15,7 @@
- - - - - - - - - - diff --git a/layout/generic/crashtests/crashtests.list b/layout/generic/crashtests/crashtests.list index 0410aad9f08e..ac8523f2b53d 100644 --- a/layout/generic/crashtests/crashtests.list +++ b/layout/generic/crashtests/crashtests.list @@ -75,7 +75,6 @@ load 368568.html load 368752.html load 368860-1.html load 368863-1.html -load 369038-1.xhtml load 369150-1.html load 369150-2.html load 369227-1.xhtml diff --git a/layout/generic/nsGfxScrollFrame.cpp b/layout/generic/nsGfxScrollFrame.cpp index 9a49a018c058..ec21c433a0bf 100644 --- a/layout/generic/nsGfxScrollFrame.cpp +++ b/layout/generic/nsGfxScrollFrame.cpp @@ -397,35 +397,6 @@ void nsHTMLScrollFrame::RemoveFrame(ChildListID aListID, nsIFrame* aOldFrame) { ReloadChildFrames(); } -static void GetScrollbarMetrics(nsBoxLayoutState& aState, nsIFrame* aBox, - nsSize* aMin, nsSize* aPref) { - NS_ASSERTION(aState.GetRenderingContext(), - "Must have rendering context in layout state for size " - "computations"); - - if (aMin) { - *aMin = aBox->GetXULMinSize(aState); - nsIFrame::AddXULMargin(aBox, *aMin); - if (aMin->width < 0) { - aMin->width = 0; - } - if (aMin->height < 0) { - aMin->height = 0; - } - } - - if (aPref) { - *aPref = aBox->GetXULPrefSize(aState); - nsIFrame::AddXULMargin(aBox, *aPref); - if (aPref->width < 0) { - aPref->width = 0; - } - if (aPref->height < 0) { - aPref->height = 0; - } - } -} - /** HTML scrolling implementation @@ -460,7 +431,6 @@ struct MOZ_STACK_CLASS ScrollReflowInput { // === Filled in by the constructor. Members in this section shouldn't change // their values after the constructor. === const ReflowInput& mReflowInput; - nsBoxLayoutState mBoxState; ShowScrollbar mHScrollbar; // If the horizontal scrollbar is allowed (even if mHScrollbar == // ShowScrollbar::Never) provided that it is for scrolling the visual viewport @@ -505,9 +475,9 @@ struct MOZ_STACK_CLASS ScrollReflowInput { ScrollReflowInput(nsHTMLScrollFrame* aFrame, const ReflowInput& aReflowInput); - nscoord VScrollbarMinHeight() const { return mVScrollbarMinSize.height; } + nscoord VScrollbarMinHeight() const { return mVScrollbarPrefSize.height; } nscoord VScrollbarPrefWidth() const { return mVScrollbarPrefSize.width; } - nscoord HScrollbarMinWidth() const { return mHScrollbarMinSize.width; } + nscoord HScrollbarMinWidth() const { return mHScrollbarPrefSize.width; } nscoord HScrollbarPrefHeight() const { return mHScrollbarPrefSize.height; } // Returns the sizes occupied by the scrollbar gutters. If aShowVScroll or @@ -515,6 +485,9 @@ struct MOZ_STACK_CLASS ScrollReflowInput { // included. nsMargin ScrollbarGutter(bool aShowVScrollbar, bool aShowHScrollbar, bool aScrollbarOnRight) const { + if (mOverlayScrollbars) { + return mScrollbarGutter; + } nsMargin gutter = mScrollbarGutter; if (aShowVScrollbar && gutter.right == 0 && gutter.left == 0) { const nscoord w = VScrollbarPrefWidth(); @@ -531,13 +504,14 @@ struct MOZ_STACK_CLASS ScrollReflowInput { return gutter; } + bool OverlayScrollbars() const { return mOverlayScrollbars; } + private: // Filled in by the constructor. Put variables here to keep them unchanged // after initializing them in the constructor. - nsSize mVScrollbarMinSize; nsSize mVScrollbarPrefSize; - nsSize mHScrollbarMinSize; nsSize mHScrollbarPrefSize; + bool mOverlayScrollbars = false; // The scrollbar gutter sizes resolved from the scrollbar-gutter and // scrollbar-width property. nsMargin mScrollbarGutter; @@ -546,55 +520,35 @@ struct MOZ_STACK_CLASS ScrollReflowInput { ScrollReflowInput::ScrollReflowInput(nsHTMLScrollFrame* aFrame, const ReflowInput& aReflowInput) : mReflowInput(aReflowInput), - // mBoxState is just used for scrollbars so we don't need to - // worry about the reflow depth here - mBoxState(aReflowInput.mFrame->PresContext(), - aReflowInput.mRenderingContext), mComputedBorder(aReflowInput.ComputedPhysicalBorderPadding() - aReflowInput.ComputedPhysicalPadding()), mScrollbarGutterFromLastReflow(aFrame->GetWritingMode()) { ScrollStyles styles = aFrame->GetScrollStyles(); mHScrollbar = ShouldShowScrollbar(styles.mHorizontal); mVScrollbar = ShouldShowScrollbar(styles.mVertical); + mOverlayScrollbars = aFrame->UsesOverlayScrollbars(); - if (nsIFrame* hScrollbarBox = aFrame->GetScrollbarBox(false)) { - nsScrollbarFrame* scrollbar = do_QueryFrame(hScrollbarBox); + if (nsScrollbarFrame* scrollbar = aFrame->GetScrollbarBox(false)) { scrollbar->SetScrollbarMediatorContent(mReflowInput.mFrame->GetContent()); - - GetScrollbarMetrics(mBoxState, hScrollbarBox, &mHScrollbarMinSize, - &mHScrollbarPrefSize); - - // A zero minimum size is a bug with non-overlay scrollbars. That - // means we'll always try to place the scrollbar, even if it will ultimately - // not fit, see bug 1809630. XUL collapsing is the exception because the + mHScrollbarPrefSize = scrollbar->ScrollbarMinSize(); + // A zero minimum size is a bug with non-overlay scrollbars. That means + // we'll always try to place the scrollbar, even if it will ultimately not + // fit, see bug 1809630. XUL collapsing is the exception because the // front-end uses it. - MOZ_ASSERT(aFrame->PresContext()->UseOverlayScrollbars() || - hScrollbarBox->IsXULCollapsed() || - (mHScrollbarMinSize.width && mHScrollbarMinSize.height), - "Shouldn't have a zero horizontal min-scrollbar-size"); - MOZ_ASSERT(mHScrollbarPrefSize.width >= mHScrollbarMinSize.width && - mHScrollbarPrefSize.height >= mHScrollbarMinSize.height, - "Scrollbar pref size should be >= min size"); - + MOZ_ASSERT(scrollbar->IsXULCollapsed() || + (mHScrollbarPrefSize.width && mHScrollbarPrefSize.height), + "Shouldn't have a zero horizontal scrollbar-size"); } else { mHScrollbar = ShowScrollbar::Never; mHScrollbarAllowedForScrollingVVInsideLV = false; } - if (nsIFrame* vScrollbarBox = aFrame->GetScrollbarBox(true)) { - nsScrollbarFrame* scrollbar = do_QueryFrame(vScrollbarBox); + if (nsScrollbarFrame* scrollbar = aFrame->GetScrollbarBox(true)) { scrollbar->SetScrollbarMediatorContent(mReflowInput.mFrame->GetContent()); - - GetScrollbarMetrics(mBoxState, vScrollbarBox, &mVScrollbarMinSize, - &mVScrollbarPrefSize); - + mVScrollbarPrefSize = scrollbar->ScrollbarMinSize(); // See above. - MOZ_ASSERT(aFrame->PresContext()->UseOverlayScrollbars() || - vScrollbarBox->IsXULCollapsed() || - (mVScrollbarMinSize.width && mVScrollbarMinSize.height), - "Shouldn't have a zero vertical min-size"); - MOZ_ASSERT(mVScrollbarPrefSize.width >= mVScrollbarMinSize.width && - mVScrollbarPrefSize.height >= mVScrollbarMinSize.height, - "Scrollbar pref size should be >= min size"); + MOZ_ASSERT(scrollbar->IsXULCollapsed() || + (mVScrollbarPrefSize.width && mVScrollbarPrefSize.height), + "Shouldn't have a zero vertical scrollbar-size"); } else { mVScrollbar = ShowScrollbar::Never; mVScrollbarAllowedForScrollingVVInsideLV = false; @@ -615,7 +569,8 @@ ScrollReflowInput::ScrollReflowInput(nsHTMLScrollFrame* aFrame, mVScrollbar = ShowScrollbar::Never; mVScrollbarAllowedForScrollingVVInsideLV = false; } else if (const auto& scrollbarGutterStyle = - scrollbarStyle->StyleDisplay()->mScrollbarGutter) { + scrollbarStyle->StyleDisplay()->mScrollbarGutter; + scrollbarGutterStyle && !mOverlayScrollbars) { const auto stable = bool(scrollbarGutterStyle & StyleScrollbarGutter::STABLE); const auto bothEdges = @@ -789,7 +744,7 @@ bool nsHTMLScrollFrame::TryLayout(ScrollReflowInput& aState, "TryLayout scrolledRect:%s overflowRect:%s scrollportSize:%s\n", ToString(scrolledRect).c_str(), ToString(overflowRect).c_str(), ToString(scrollPortSize).c_str()); - nscoord oneDevPixel = aState.mBoxState.PresContext()->DevPixelsToAppUnits(1); + nscoord oneDevPixel = PresContext()->DevPixelsToAppUnits(1); bool showHScrollbar = aAssumeHScroll; bool showVScrollbar = aAssumeVScroll; @@ -853,6 +808,7 @@ bool nsHTMLScrollFrame::TryLayout(ScrollReflowInput& aState, if (mIsRoot && gfxPlatform::UseDesktopZoomingScrollbars()) { bool vvChanged = true; + const bool overlay = aState.OverlayScrollbars(); // This loop can run at most twice since we can only add a scrollbar once. // At this point we've already decided that this layout is consistent so we // will return true. Scrollbars added here never take up layout space even @@ -863,9 +819,12 @@ bool nsHTMLScrollFrame::TryLayout(ScrollReflowInput& aState, if (!aState.mShowHScrollbar && aState.mHScrollbarAllowedForScrollingVVInsideLV) { if (ScrollPort().width >= visualViewportSize.width + oneDevPixel && - visualViewportSize.width >= aState.HScrollbarMinWidth()) { + (overlay || + visualViewportSize.width >= aState.HScrollbarMinWidth())) { vvChanged = true; - visualViewportSize.height -= aState.HScrollbarPrefHeight(); + if (!overlay) { + visualViewportSize.height -= aState.HScrollbarPrefHeight(); + } aState.mShowHScrollbar = true; aState.mOnlyNeedHScrollbarToScrollVVInsideLV = true; ROOT_SCROLLBAR_LOG("TryLayout added H scrollbar for VV, VV now %s\n", @@ -876,9 +835,12 @@ bool nsHTMLScrollFrame::TryLayout(ScrollReflowInput& aState, if (!aState.mShowVScrollbar && aState.mVScrollbarAllowedForScrollingVVInsideLV) { if (ScrollPort().height >= visualViewportSize.height + oneDevPixel && - visualViewportSize.height >= aState.VScrollbarMinHeight()) { + (overlay || + visualViewportSize.height >= aState.VScrollbarMinHeight())) { vvChanged = true; - visualViewportSize.width -= aState.VScrollbarPrefWidth(); + if (!overlay) { + visualViewportSize.width -= aState.VScrollbarPrefWidth(); + } aState.mShowVScrollbar = true; aState.mOnlyNeedVScrollbarToScrollVVInsideLV = true; ROOT_SCROLLBAR_LOG("TryLayout added V scrollbar for VV, VV now %s\n", @@ -1239,13 +1201,17 @@ void nsHTMLScrollFrame::PlaceScrollArea(ScrollReflowInput& aState, nscoord nsHTMLScrollFrame::IntrinsicScrollbarGutterSizeAtInlineEdges( gfxContext* aRenderingContext) { const bool isVerticalWM = GetWritingMode().IsVertical(); - nsIFrame* inlineEndScrollbarBox = + nsScrollbarFrame* inlineEndScrollbarBox = isVerticalWM ? mHScrollbarBox : mVScrollbarBox; if (!inlineEndScrollbarBox) { // No scrollbar box frame means no intrinsic size. return 0; } + if (PresContext()->UseOverlayScrollbars()) { + return 0; + } + const auto* styleForScrollbar = nsLayoutUtils::StyleForScrollbar(this); if (styleForScrollbar->StyleUIReset()->ScrollbarWidth() == StyleScrollbarWidth::None) { @@ -1268,9 +1234,7 @@ nscoord nsHTMLScrollFrame::IntrinsicScrollbarGutterSizeAtInlineEdges( } // No need to worry about reflow depth here since it's just for scrollbars. - nsBoxLayoutState bls(PresContext(), aRenderingContext, 0); - nsSize scrollbarPrefSize; - GetScrollbarMetrics(bls, inlineEndScrollbarBox, nullptr, &scrollbarPrefSize); + nsSize scrollbarPrefSize = inlineEndScrollbarBox->ScrollbarMinSize(); const nscoord scrollbarSize = isVerticalWM ? scrollbarPrefSize.height : scrollbarPrefSize.width; const auto bothEdges = @@ -1608,7 +1572,7 @@ void nsHTMLScrollFrame::Reflow(nsPresContext* aPresContext, const nsRect insideBorderArea( nsPoint(state.mComputedBorder.left, state.mComputedBorder.top), layoutSize); - LayoutScrollbars(state.mBoxState, insideBorderArea, oldScrollPort); + LayoutScrollbars(state, insideBorderArea, oldScrollPort); } else { mSkippedScrollbarLayout = true; } @@ -1638,7 +1602,7 @@ void nsHTMLScrollFrame::Reflow(nsPresContext* aPresContext, // Note that we need to do this after the // UpdateVisualViewportSizeForPotentialScrollbarChange call above because that // is what updates the visual viewport size and we need it to be up to date. - if (mIsRoot && !UsesOverlayScrollbars() && + if (mIsRoot && !state.OverlayScrollbars() && (didHaveHScrollbar != state.mShowHScrollbar || didHaveVScrollbar != state.mShowVScrollbar || didOnlyHScrollbar != mOnlyNeedHScrollbarToScrollVVInsideLV || @@ -1878,7 +1842,7 @@ void nsHTMLScrollFrame::ScrollByWhole(nsScrollbarFrame* aScrollbar, void nsHTMLScrollFrame::ScrollByLine(nsScrollbarFrame* aScrollbar, int32_t aDirection, ScrollSnapFlags aSnapFlags) { - bool isHorizontal = aScrollbar->IsXULHorizontal(); + bool isHorizontal = aScrollbar->IsHorizontal(); nsIntPoint delta; if (isHorizontal) { const double kScrollMultiplier = @@ -1916,7 +1880,7 @@ void nsHTMLScrollFrame::RepeatButtonScroll(nsScrollbarFrame* aScrollbar) { void nsHTMLScrollFrame::ThumbMoved(nsScrollbarFrame* aScrollbar, nscoord aOldPos, nscoord aNewPos) { MOZ_ASSERT(aScrollbar != nullptr); - bool isHorizontal = aScrollbar->IsXULHorizontal(); + bool isHorizontal = aScrollbar->IsHorizontal(); nsPoint current = GetScrollPosition(); nsPoint dest = current; if (isHorizontal) { @@ -1953,7 +1917,7 @@ void nsHTMLScrollFrame::ScrollByUnit(nsScrollbarFrame* aScrollbar, ScrollUnit aUnit, ScrollSnapFlags aSnapFlags) { MOZ_ASSERT(aScrollbar != nullptr); - bool isHorizontal = aScrollbar->IsXULHorizontal(); + bool isHorizontal = aScrollbar->IsHorizontal(); nsIntPoint delta; if (isHorizontal) { delta.x = aDirection; @@ -5468,10 +5432,12 @@ void nsHTMLScrollFrame::ReloadChildFrames() { if (value.LowerCaseEqualsLiteral("horizontal")) { NS_ASSERTION(!mHScrollbarBox, "Found multiple horizontal scrollbars?"); - mHScrollbarBox = frame; + mHScrollbarBox = do_QueryFrame(frame); + MOZ_ASSERT(mHScrollbarBox, "Not a scrollbar?"); } else { NS_ASSERTION(!mVScrollbarBox, "Found multiple vertical scrollbars?"); - mVScrollbarBox = frame; + mVScrollbarBox = do_QueryFrame(frame); + MOZ_ASSERT(mVScrollbarBox, "Not a scrollbar?"); } } else if (content->IsXULElement(nsGkAtoms::resizer)) { NS_ASSERTION(!mResizerBox, "Found multiple resizers"); @@ -5817,7 +5783,9 @@ void nsHTMLScrollFrame::CurPosAttributeChangedInternal(nsIContent* aContent, // // In cases 2 and 3 we do not need to scroll because we're just // updating our scrollbar. - if (mFrameIsUpdatingScrollbar) return; + if (mFrameIsUpdatingScrollbar) { + return; + } nsRect scrollRange = GetVisualScrollRange(); @@ -6530,12 +6498,39 @@ static void AdjustOverlappingScrollbars(nsRect& aVRect, nsRect& aHRect) { } } -void nsHTMLScrollFrame::LayoutScrollbars(nsBoxLayoutState& aState, +void nsHTMLScrollFrame::LayoutScrollbarPartAtRect( + const ScrollReflowInput& aState, ReflowInput& aKidReflowInput, + const nsRect& aRect) { + nsPresContext* pc = PresContext(); + nsIFrame* kid = aKidReflowInput.mFrame; + const auto wm = kid->GetWritingMode(); + ReflowOutput desiredSize(wm); + MOZ_ASSERT(!wm.IsVertical(), + "Scrollbar parts should have writing-mode: initial"); + MOZ_ASSERT(!wm.IsInlineReversed(), + "Scrollbar parts should have writing-mode: initial"); + // XXX Maybe get a meaningful container size or something. Shouldn't matter + // given our asserts above. + const nsSize containerSize; + aKidReflowInput.SetComputedISize(aRect.Width()); + aKidReflowInput.SetComputedBSize(aRect.Height()); + + const LogicalPoint pos(wm, aRect.TopLeft(), containerSize); + const auto flags = ReflowChildFlags::Default; + nsReflowStatus status; + ReflowOutput kidDesiredSize(wm); + ReflowChild(kid, pc, kidDesiredSize, aKidReflowInput, wm, pos, containerSize, + flags, status); + FinishReflowChild(kid, pc, kidDesiredSize, &aKidReflowInput, wm, pos, + containerSize, flags); +} + +void nsHTMLScrollFrame::LayoutScrollbars(ScrollReflowInput& aState, const nsRect& aInsideBorderArea, const nsRect& aOldScrollPort) { NS_ASSERTION(!mSuppressScrollbarUpdate, "This should have been suppressed"); - bool scrollbarOnLeft = !IsScrollbarOnRight(); + const bool scrollbarOnLeft = !IsScrollbarOnRight(); const bool overlayScrollbars = UsesOverlayScrollbars(); const bool overlayScrollBarsOnRoot = overlayScrollbars && mIsRoot; const bool showVScrollbar = mVScrollbarBox && mHasVerticalScrollbar; @@ -6550,7 +6545,6 @@ void nsHTMLScrollFrame::LayoutScrollbars(nsBoxLayoutState& aState, nsPresContext* presContext = mScrolledFrame->PresContext(); nsRect vRect; if (showVScrollbar) { - MOZ_ASSERT(mVScrollbarBox->IsXULBoxFrame(), "Must be a box frame!"); vRect.height = overlayScrollBarsOnRoot ? compositionSize.height : mScrollPort.height; vRect.y = mScrollPort.y; @@ -6561,67 +6555,46 @@ void nsHTMLScrollFrame::LayoutScrollbars(nsBoxLayoutState& aState, vRect.width = aInsideBorderArea.XMost() - mScrollPort.XMost(); vRect.x = mScrollPort.x + compositionSize.width; } - - // For overlay scrollbars the margin returned from this GetXULMargin call - // is a negative margin that moves the scrollbar from just outside the - // scrollport (and hence not visible) to just inside the scrollport (and - // hence visible). For non-overlay scrollbars it is a 0 margin. - nsMargin margin; - mVScrollbarBox->GetXULMargin(margin); - - if (!overlayScrollbars && mOnlyNeedVScrollbarToScrollVVInsideLV) { + if (overlayScrollbars || mOnlyNeedVScrollbarToScrollVVInsideLV) { + const nscoord width = aState.VScrollbarPrefWidth(); // There is no space reserved for the layout scrollbar, it is currently // not visible because it is positioned just outside the scrollport. But // we know that it needs to be made visible so we shift it back in. - nsSize vScrollbarPrefSize(0, 0); - GetScrollbarMetrics(aState, mVScrollbarBox, nullptr, &vScrollbarPrefSize); + vRect.width += width; if (scrollbarOnLeft) { - margin.right -= vScrollbarPrefSize.width; + vRect.x += width; } else { - margin.left -= vScrollbarPrefSize.width; + vRect.x -= width; } } - - vRect.Deflate(margin); } nsRect hRect; if (showHScrollbar) { - MOZ_ASSERT(mHScrollbarBox->IsXULBoxFrame(), "Must be a box frame!"); hRect.width = overlayScrollBarsOnRoot ? compositionSize.width : mScrollPort.width; hRect.x = mScrollPort.x; hRect.height = aInsideBorderArea.YMost() - mScrollPort.YMost(); hRect.y = mScrollPort.y + compositionSize.height; - // For overlay scrollbars the margin returned from this GetXULMargin call - // is a negative margin that moves the scrollbar from just outside the - // scrollport (and hence not visible) to just inside the scrollport (and - // hence visible). For non-overlay scrollbars it is a 0 margin. - nsMargin margin; - mHScrollbarBox->GetXULMargin(margin); - - if (!overlayScrollbars && mOnlyNeedHScrollbarToScrollVVInsideLV) { + if (overlayScrollbars || mOnlyNeedHScrollbarToScrollVVInsideLV) { + const nscoord height = aState.HScrollbarPrefHeight(); + hRect.height += height; // There is no space reserved for the layout scrollbar, it is currently // not visible because it is positioned just outside the scrollport. But // we know that it needs to be made visible so we shift it back in. - nsSize hScrollbarPrefSize(0, 0); - GetScrollbarMetrics(aState, mHScrollbarBox, nullptr, &hScrollbarPrefSize); - margin.top -= hScrollbarPrefSize.height; + hRect.y -= height; } - - hRect.Deflate(margin); } const bool hasVisualOnlyScrollbarsOnBothDirections = !overlayScrollbars && showHScrollbar && mOnlyNeedHScrollbarToScrollVVInsideLV && showVScrollbar && mOnlyNeedVScrollbarToScrollVVInsideLV; + nsPresContext* pc = PresContext(); // place the scrollcorner if (mScrollCornerBox) { - MOZ_ASSERT(mScrollCornerBox->IsXULBoxFrame(), "Must be a box frame!"); - nsRect r(0, 0, 0, 0); if (scrollbarOnLeft) { // scrollbar (if any) on left @@ -6653,20 +6626,25 @@ void nsHTMLScrollFrame::LayoutScrollbars(nsBoxLayoutState& aState, r.y = mScrollPort.YMost() - r.height; } - nsBoxFrame::LayoutChildAt(aState, mScrollCornerBox, r); + ReflowInput scrollCornerRI( + pc, aState.mReflowInput, mScrollCornerBox, + LogicalSize(mScrollCornerBox->GetWritingMode(), r.Size())); + LayoutScrollbarPartAtRect(aState, scrollCornerRI, r); } if (mResizerBox) { // If a resizer is present, get its size. // // TODO(emilio): Should this really account for scrollbar-width? - nsPresContext* pc = aState.PresContext(); auto scrollbarWidth = nsLayoutUtils::StyleForScrollbar(this) ->StyleUIReset() ->ScrollbarWidth(); auto scrollbarSize = pc->Theme()->GetScrollbarSize(pc, scrollbarWidth, nsITheme::Overlay::No); - nsSize resizerMinSize = mResizerBox->GetXULMinSize(aState); + ReflowInput resizerRI(pc, aState.mReflowInput, mResizerBox, + LogicalSize(mResizerBox->GetWritingMode())); + nsSize resizerMinSize = {resizerRI.ComputedMinWidth(), + resizerRI.ComputedMinHeight()}; nsRect r; nscoord vScrollbarWidth = pc->DevPixelsToAppUnits(scrollbarSize); @@ -6680,7 +6658,7 @@ void nsHTMLScrollFrame::LayoutScrollbars(nsBoxLayoutState& aState, std::max(std::max(r.height, hScrollbarHeight), resizerMinSize.height); r.y = aInsideBorderArea.YMost() - r.height; - nsBoxFrame::LayoutChildAt(aState, mResizerBox, r); + LayoutScrollbarPartAtRect(aState, resizerRI, r); } // Note that AdjustScrollbarRectForResizer has to be called after the @@ -6702,10 +6680,16 @@ void nsHTMLScrollFrame::LayoutScrollbars(nsBoxLayoutState& aState, AdjustOverlappingScrollbars(vRect, hRect); } if (mVScrollbarBox) { - nsBoxFrame::LayoutChildAt(aState, mVScrollbarBox, vRect); + ReflowInput vScrollbarRI( + pc, aState.mReflowInput, mVScrollbarBox, + LogicalSize(mVScrollbarBox->GetWritingMode(), vRect.Size())); + LayoutScrollbarPartAtRect(aState, vScrollbarRI, vRect); } if (mHScrollbarBox) { - nsBoxFrame::LayoutChildAt(aState, mHScrollbarBox, hRect); + ReflowInput hScrollbarRI( + pc, aState.mReflowInput, mHScrollbarBox, + LogicalSize(mHScrollbarBox->GetWritingMode(), hRect.Size())); + LayoutScrollbarPartAtRect(aState, hScrollbarRI, hRect); } // may need to update fixed position children of the viewport, @@ -6720,7 +6704,7 @@ void nsHTMLScrollFrame::LayoutScrollbars(nsBoxLayoutState& aState, // post reflow callback to modify scrollbar attributes mUpdateScrollbarAttributes = true; if (!mPostedReflowCallback) { - aState.PresShell()->PostReflowCallback(this); + PresShell()->PostReflowCallback(this); mPostedReflowCallback = true; } } diff --git a/layout/generic/nsGfxScrollFrame.h b/layout/generic/nsGfxScrollFrame.h index 542146353f52..effa382a9b0b 100644 --- a/layout/generic/nsGfxScrollFrame.h +++ b/layout/generic/nsGfxScrollFrame.h @@ -385,7 +385,7 @@ class nsHTMLScrollFrame : public nsContainerFrame, nscoord aNewPos) final; void ScrollbarReleased(nsScrollbarFrame* aScrollbar) final; void VisibilityChanged(bool aVisible) final {} - nsIFrame* GetScrollbarBox(bool aVertical) final { + nsScrollbarFrame* GetScrollbarBox(bool aVertical) final { return aVertical ? mVScrollbarBox : mHScrollbarBox; } @@ -717,10 +717,13 @@ class nsHTMLScrollFrame : public nsContainerFrame, void AdjustScrollbarRectForResizer( nsIFrame* aFrame, nsPresContext* aPresContext, nsRect& aRect, bool aHasResizer, mozilla::layers::ScrollDirection aDirection); - void LayoutScrollbars(nsBoxLayoutState& aState, + void LayoutScrollbars(ScrollReflowInput& aState, const nsRect& aInsideBorderArea, const nsRect& aOldScrollPort); + void LayoutScrollbarPartAtRect(const ScrollReflowInput&, + ReflowInput& aKidReflowInput, const nsRect&); + bool IsAlwaysActive() const; void MarkRecentlyScrolled(); void MarkNotRecentlyScrolled(); @@ -783,8 +786,8 @@ class nsHTMLScrollFrame : public nsContainerFrame, RefPtr mScrollEndEvent; nsRevocableEventPtr mAsyncScrollPortEvent; nsRevocableEventPtr mScrolledAreaEvent; - nsIFrame* mHScrollbarBox; - nsIFrame* mVScrollbarBox; + nsScrollbarFrame* mHScrollbarBox; + nsScrollbarFrame* mVScrollbarBox; nsIFrame* mScrolledFrame; nsIFrame* mScrollCornerBox; nsIFrame* mResizerBox; diff --git a/layout/generic/nsInlineFrame.cpp b/layout/generic/nsInlineFrame.cpp index e318fecd038e..4c03d8fca031 100644 --- a/layout/generic/nsInlineFrame.cpp +++ b/layout/generic/nsInlineFrame.cpp @@ -275,7 +275,7 @@ void nsInlineFrame::Reflow(nsPresContext* aPresContext, ReflowOutput& aMetrics, DISPLAY_REFLOW(aPresContext, this, aReflowInput, aMetrics, aStatus); MOZ_ASSERT(aStatus.IsEmpty(), "Caller should pass a fresh reflow status!"); - if (nullptr == aReflowInput.mLineLayout) { + if (!aReflowInput.mLineLayout) { NS_ERROR("must have non-null aReflowInput.mLineLayout"); return; } diff --git a/layout/style/res/scrollbars.css b/layout/style/res/scrollbars.css index b16b1fac5eb7..e3992b0cdc16 100644 --- a/layout/style/res/scrollbars.css +++ b/layout/style/res/scrollbars.css @@ -21,24 +21,15 @@ -moz-font-smoothing-background-color: initial; -moz-min-font-size-ratio: initial; -moz-box-collapse: initial; - - /* -moz-box-layout: initial; is not included in 'all' but it's not needed, - * because we explicitly specify it below for the top level scrollbar parts - * (and it inherits through to the rest). - */ + -moz-box-layout: initial; math-depth: initial; /* As long as inert implies pointer-events: none as it does now, we're * good. */ -moz-inert: initial; - /* Using initial is not sufficient for direction, since its initial value can - * depend on the document's language. - * - * LTR is what we want for all scrollbar parts anyway, so that e.g. we don't - * reverse the rendering of a horizontal scrollbar. - */ - direction: ltr; + /* direction: initial is not sufficient, since its initial value can depend + * on the document's language. But we specify ltr explicitly below */ /* Similarly for font properties, whose initial values depend on the * document's language. Scrollbar parts don't have any text or rely on @@ -69,16 +60,17 @@ } scrollbar, scrollbarbutton, scrollcorner, slider, thumb, resizer { - /* Force legacy XUL layout for now on scrollbars / resizers and descendants, - * as nsHTMLScrollFrame relies on that (and nsScrollbarFrame is a XUL frame - * anyways). - * - * TODO: Eventually we should rewrite scrollbars so that they don't use XUL - * layout. - */ - -moz-box-layout: legacy; - display: -moz-box; + /* We need a display value that doesn't get blockified to preserve the + * scrollbar sizing asserts. In practice it doesn't matter since these get + * special frames */ + display: block; box-sizing: border-box; + + /* Our scrollbar layout uses physical coordinates, we wouldn't want an + * horizontal scrollbar to flip in rtl for example. */ + direction: ltr; + writing-mode: initial; + -moz-user-focus: ignore; /* Prevent -moz-user-modify declaration from designmode.css having an effect. */ -moz-user-modify: initial; @@ -96,11 +88,6 @@ resizer { appearance: auto; -moz-default-appearance: resizer; - /* native resizer should never flip on its own; - we will flip it (or the SVG background) with CSS transform below. */ - direction: ltr; - writing-mode: initial; - background: url("chrome://global/skin/icons/resizer.svg") no-repeat; background-size: 100% 100%; cursor: se-resize; @@ -147,14 +134,10 @@ resizer[dir="topright"] { thumb { appearance: auto; -moz-default-appearance: scrollbarthumb-horizontal; - -moz-box-flex: 1; - -moz-box-align: center; - -moz-box-pack: center; } thumb[orient="vertical"] { -moz-default-appearance: scrollbarthumb-vertical; - -moz-box-orient: vertical; } scrollbar[disabled="true"] thumb { @@ -175,7 +158,6 @@ scrollbar { scrollbar[orient="vertical"] { -moz-default-appearance: scrollbar-vertical; - -moz-box-orient: vertical; } scrollbar[root="true"] { @@ -197,12 +179,10 @@ scrollbar[root="true"] { slider { appearance: auto; -moz-default-appearance: scrollbartrack-horizontal; - -moz-box-flex: 1; } slider[orient="vertical"] { -moz-default-appearance: scrollbartrack-vertical; - -moz-box-orient: vertical; } scrollbarbutton { diff --git a/layout/style/test/test_viewport_scrollbar_causing_reflow.html b/layout/style/test/test_viewport_scrollbar_causing_reflow.html index 40560d73183a..ced14f352a67 100644 --- a/layout/style/test/test_viewport_scrollbar_causing_reflow.html +++ b/layout/style/test/test_viewport_scrollbar_causing_reflow.html @@ -13,18 +13,21 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1367568 Mozilla Bug 1367568
-
- fixed-width -
(child)
-
+ changes. More than 5 so that our leeway for scrollbar parts doesn't + accidentally cause the test to pass --> +
fixed-width
(child)
+
fixed-width
(child)
+
fixed-width
(child)
+
fixed-width
(child)
+
fixed-width
(child)
+
fixed-width
(child)
abs-fixed-width
(child)
-