From cdad6a0be84298698ab7d785b8d0e714e05175e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 3 Mar 2020 18:50:07 +0000 Subject: [PATCH] Bug 1619428 - Make the overlay scrollbars check work in all platforms. r=mstange Reuse the AddXULMinSize logic which already deals with all the widget stuff, non-themed scrollbars, etc. Remove some useless margin declarations and such in GeckoView scrollbars code now that AddXULMinSize does look at the min-width/height properties. Differential Revision: https://phabricator.services.mozilla.com/D65129 --HG-- extra : moz-landing-system : lando --- layout/generic/nsFrame.cpp | 2 +- layout/generic/nsGfxScrollFrame.cpp | 2 +- layout/generic/nsIFrame.h | 4 +- layout/xul/grid/nsGrid.cpp | 2 +- layout/xul/nsBox.cpp | 16 +++---- layout/xul/nsBoxFrame.cpp | 3 +- layout/xul/nsImageBoxFrame.cpp | 2 +- layout/xul/nsScrollbarFrame.cpp | 46 ++++++++++++--------- layout/xul/nsTextBoxFrame.cpp | 2 +- layout/xul/tree/nsTreeBodyFrame.cpp | 2 +- toolkit/themes/mobile/global/scrollbars.css | 4 -- 11 files changed, 45 insertions(+), 40 deletions(-) diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp index 49a8a620cf3c..bcce3c17c88b 100644 --- a/layout/generic/nsFrame.cpp +++ b/layout/generic/nsFrame.cpp @@ -10408,7 +10408,7 @@ nsSize nsFrame::GetXULMinSize(nsBoxLayoutState& aState) { // get our size in CSS. bool widthSet, heightSet; bool completelyRedefined = - nsIFrame::AddXULMinSize(aState, this, size, widthSet, heightSet); + nsIFrame::AddXULMinSize(this, size, widthSet, heightSet); // Refresh our caches with new sizes. if (!completelyRedefined) { diff --git a/layout/generic/nsGfxScrollFrame.cpp b/layout/generic/nsGfxScrollFrame.cpp index 18aad0692620..becd5b51d4f6 100644 --- a/layout/generic/nsGfxScrollFrame.cpp +++ b/layout/generic/nsGfxScrollFrame.cpp @@ -1665,7 +1665,7 @@ nsSize nsXULScrollFrame::GetXULMinSize(nsBoxLayoutState& aState) { AddBorderAndPadding(min); bool widthSet, heightSet; - nsIFrame::AddXULMinSize(aState, this, min, widthSet, heightSet); + nsIFrame::AddXULMinSize(this, min, widthSet, heightSet); return min; } diff --git a/layout/generic/nsIFrame.h b/layout/generic/nsIFrame.h index 402030fd3412..ca7afd4ae012 100644 --- a/layout/generic/nsIFrame.h +++ b/layout/generic/nsIFrame.h @@ -3848,8 +3848,8 @@ class nsIFrame : public nsQueryFrame { static bool AddXULPrefSize(nsIFrame* aBox, nsSize& aSize, bool& aWidth, bool& aHeightSet); - static bool AddXULMinSize(nsBoxLayoutState& aState, nsIFrame* aBox, - nsSize& aSize, bool& aWidth, bool& aHeightSet); + static bool AddXULMinSize(nsIFrame* aBox, nsSize& aSize, bool& aWidth, + bool& aHeightSet); static bool AddXULMaxSize(nsIFrame* aBox, nsSize& aSize, bool& aWidth, bool& aHeightSet); static bool AddXULFlex(nsIFrame* aBox, nscoord& aFlex); diff --git a/layout/xul/grid/nsGrid.cpp b/layout/xul/grid/nsGrid.cpp index 95a9a8012a03..ebcc35aa36c6 100644 --- a/layout/xul/grid/nsGrid.cpp +++ b/layout/xul/grid/nsGrid.cpp @@ -796,7 +796,7 @@ nscoord nsGrid::GetMinRowHeight(nsBoxLayoutState& aState, int32_t aIndex, if (box) { bool widthSet, heightSet; nsSize cssSize(-1, -1); - nsIFrame::AddXULMinSize(aState, box, cssSize, widthSet, heightSet); + nsIFrame::AddXULMinSize(box, cssSize, widthSet, heightSet); row->mMin = GET_HEIGHT(cssSize, aIsHorizontal); diff --git a/layout/xul/nsBox.cpp b/layout/xul/nsBox.cpp index ab3a807c40e6..675cba7f9ece 100644 --- a/layout/xul/nsBox.cpp +++ b/layout/xul/nsBox.cpp @@ -229,7 +229,7 @@ nsSize nsBox::GetXULMinSize(nsBoxLayoutState& aState) { AddBorderAndPadding(min); bool widthSet, heightSet; - nsIFrame::AddXULMinSize(aState, this, min, widthSet, heightSet); + nsIFrame::AddXULMinSize(this, min, widthSet, heightSet); return min; } @@ -421,28 +421,30 @@ static nscoord GetScrollbarWidthNoTheme(nsIFrame* aBox) { } } -bool nsIFrame::AddXULMinSize(nsBoxLayoutState& aState, nsIFrame* aBox, +bool nsIFrame::AddXULMinSize(nsIFrame* aBox, nsSize& aSize, bool& aWidthSet, bool& aHeightSet) { aWidthSet = false; aHeightSet = false; bool canOverride = true; + nsPresContext* pc = aBox->PresContext(); + // See if a native theme wants to supply a minimum size. const nsStyleDisplay* display = aBox->StyleDisplay(); if (display->HasAppearance()) { - nsITheme* theme = aState.PresContext()->GetTheme(); - if (theme && theme->ThemeSupportsWidget(aState.PresContext(), aBox, + nsITheme* theme = pc->GetTheme(); + if (theme && theme->ThemeSupportsWidget(pc, aBox, display->mAppearance)) { LayoutDeviceIntSize size; - theme->GetMinimumWidgetSize(aState.PresContext(), aBox, + theme->GetMinimumWidgetSize(pc, aBox, display->mAppearance, &size, &canOverride); if (size.width) { - aSize.width = aState.PresContext()->DevPixelsToAppUnits(size.width); + aSize.width = pc->DevPixelsToAppUnits(size.width); aWidthSet = true; } if (size.height) { - aSize.height = aState.PresContext()->DevPixelsToAppUnits(size.height); + aSize.height = pc->DevPixelsToAppUnits(size.height); aHeightSet = true; } } else { diff --git a/layout/xul/nsBoxFrame.cpp b/layout/xul/nsBoxFrame.cpp index b55f55a11fa3..eb119cd35d6a 100644 --- a/layout/xul/nsBoxFrame.cpp +++ b/layout/xul/nsBoxFrame.cpp @@ -606,8 +606,7 @@ nsSize nsBoxFrame::GetXULMinSize(nsBoxLayoutState& aBoxLayoutState) { // if the size was not completely redefined in CSS then ask our children bool widthSet, heightSet; - if (!nsIFrame::AddXULMinSize(aBoxLayoutState, this, size, widthSet, - heightSet)) { + if (!nsIFrame::AddXULMinSize(this, size, widthSet, heightSet)) { if (mLayoutManager) { nsSize layoutSize = mLayoutManager->GetXULMinSize(this, aBoxLayoutState); if (!widthSet) size.width = layoutSize.width; diff --git a/layout/xul/nsImageBoxFrame.cpp b/layout/xul/nsImageBoxFrame.cpp index ceb9753ec6b8..8d3df8718a64 100644 --- a/layout/xul/nsImageBoxFrame.cpp +++ b/layout/xul/nsImageBoxFrame.cpp @@ -736,7 +736,7 @@ nsSize nsImageBoxFrame::GetXULMinSize(nsBoxLayoutState& aState) { DISPLAY_MIN_SIZE(this, size); AddBorderAndPadding(size); bool widthSet, heightSet; - nsIFrame::AddXULMinSize(aState, this, size, widthSet, heightSet); + nsIFrame::AddXULMinSize(this, size, widthSet, heightSet); return size; } diff --git a/layout/xul/nsScrollbarFrame.cpp b/layout/xul/nsScrollbarFrame.cpp index 3fdc025a794c..e1f4e74fb74b 100644 --- a/layout/xul/nsScrollbarFrame.cpp +++ b/layout/xul/nsScrollbarFrame.cpp @@ -163,39 +163,47 @@ nsIScrollbarMediator* nsScrollbarFrame::GetScrollbarMediator() { } nsresult nsScrollbarFrame::GetXULMargin(nsMargin& aMargin) { - nsresult rv = NS_ERROR_FAILURE; aMargin.SizeTo(0, 0, 0, 0); - if (LookAndFeel::GetInt(LookAndFeel::eIntID_UseOverlayScrollbars) != 0) { - nsPresContext* presContext = PresContext(); - nsITheme* theme = presContext->GetTheme(); - if (theme && theme->ThemeSupportsWidget(presContext, this, - StyleAppearance::Scrollbar)) { - LayoutDeviceIntSize size; - bool isOverridable; - theme->GetMinimumWidgetSize(presContext, this, StyleAppearance::Scrollbar, - &size, &isOverridable); - if (IsXULHorizontal()) { - aMargin.top = -presContext->DevPixelsToAppUnits(size.height); - } else { - aMargin.left = -presContext->DevPixelsToAppUnits(size.width); + const bool overlayScrollbars = + !!LookAndFeel::GetInt(LookAndFeel::eIntID_UseOverlayScrollbars); + + const bool horizontal = IsXULHorizontal(); + bool didSetMargin = false; + + if (overlayScrollbars) { + nsSize minSize; + bool widthSet = false; + bool heightSet = false; + AddXULMinSize(this, minSize, widthSet, heightSet); + if (horizontal) { + if (heightSet) { + aMargin.top = -minSize.height; + didSetMargin = true; + } + } else { + if (widthSet) { + aMargin.left = -minSize.width; + didSetMargin = true; } - rv = NS_OK; } } - if (NS_FAILED(rv)) { - rv = nsBox::GetXULMargin(aMargin); + if (!didSetMargin) { + DebugOnly rv = nsBox::GetXULMargin(aMargin); + // TODO(emilio): Should probably not be fallible, it's not like anybody + // cares about the return value anyway. + MOZ_ASSERT(NS_SUCCEEDED(rv), "nsBox::GetXULMargin can't really fail"); } - if (NS_SUCCEEDED(rv) && !IsXULHorizontal()) { + if (!horizontal) { nsIScrollbarMediator* scrollFrame = GetScrollbarMediator(); if (scrollFrame && !scrollFrame->IsScrollbarOnRight()) { std::swap(aMargin.left, aMargin.right); } } - return rv; + return NS_OK; } void nsScrollbarFrame::SetIncrementToLine(int32_t aDirection) { diff --git a/layout/xul/nsTextBoxFrame.cpp b/layout/xul/nsTextBoxFrame.cpp index cf970d27af36..eaba4e730392 100644 --- a/layout/xul/nsTextBoxFrame.cpp +++ b/layout/xul/nsTextBoxFrame.cpp @@ -1080,7 +1080,7 @@ nsSize nsTextBoxFrame::GetXULMinSize(nsBoxLayoutState& aBoxLayoutState) { AddBorderAndPadding(size); bool widthSet, heightSet; - nsIFrame::AddXULMinSize(aBoxLayoutState, this, size, widthSet, heightSet); + nsIFrame::AddXULMinSize(this, size, widthSet, heightSet); return size; } diff --git a/layout/xul/tree/nsTreeBodyFrame.cpp b/layout/xul/tree/nsTreeBodyFrame.cpp index f2f9ed7d7fe5..57e398a88821 100644 --- a/layout/xul/tree/nsTreeBodyFrame.cpp +++ b/layout/xul/tree/nsTreeBodyFrame.cpp @@ -192,7 +192,7 @@ nsSize nsTreeBodyFrame::GetXULMinSize(nsBoxLayoutState& aBoxLayoutState) { AddBorderAndPadding(min); bool widthSet, heightSet; - nsIFrame::AddXULMinSize(aBoxLayoutState, this, min, widthSet, heightSet); + nsIFrame::AddXULMinSize(this, min, widthSet, heightSet); return min; } diff --git a/toolkit/themes/mobile/global/scrollbars.css b/toolkit/themes/mobile/global/scrollbars.css index ec28994f85fb..91240a19c977 100644 --- a/toolkit/themes/mobile/global/scrollbars.css +++ b/toolkit/themes/mobile/global/scrollbars.css @@ -20,10 +20,7 @@ xul|scrollbar { display: block; } -/* Scrollbar code will reset the margin to the correct side depending on - where layout actually puts the scrollbar */ xul|scrollbar[orient="vertical"] { - margin-left: -6px; min-width: 6px; max-width: 6px; } @@ -35,7 +32,6 @@ xul|scrollbar[orient="vertical"] xul|thumb { } xul|scrollbar[orient="horizontal"] { - margin-top: -6px; min-height: 6px; max-height: 6px; }