Bug 1365806 - Make scrollbars not impose a minimum size on the scroller. r=dholbert

Also, don't suppress scrollbars if the scrollport is less than their length, as that can happen after this patch :) Differential Revision: https://phabricator.services.mozilla.com/D73395
2020-05-06 09:46:20 +00:00 · 2020-05-06 09:46:20 +00:00 · 0d476766fc
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@ -385,10 +385,7 @@ static void GetScrollbarMetrics(nsBoxLayoutState& aState, nsIFrame* aBox,
 * 1) the style is not HIDDEN
 * 2) our inside-border height is at least the scrollbar height (i.e., the
 * scrollbar fits vertically)
- * 3) our scrollport width (the inside-border width minus the width allocated
- * for a vertical scrollbar, if showing) is at least the scrollbar's min-width
- * (i.e., the scrollbar fits horizontally)
- * 4) the style is SCROLL, or the kid's overflow-area XMost is
+ * 3) the style is SCROLL, or the kid's overflow-area XMost is
 * greater than the scrollport width
 *
 * @param aForce if true, then we just assume the layout is consistent.
@ -414,31 +411,28 @@ bool nsHTMLScrollFrame::TryLayout(ScrollReflowInput* aState,
    ReflowScrolledFrame(aState, aAssumeHScroll, aAssumeVScroll, aKidMetrics);
  }

-  nsSize vScrollbarMinSize(0, 0);
  nsSize vScrollbarPrefSize(0, 0);
  if (mHelper.mVScrollbarBox) {
    GetScrollbarMetrics(aState->mBoxState, mHelper.mVScrollbarBox,
-                        &vScrollbarMinSize,
+                        nullptr,
                        aAssumeVScroll ? &vScrollbarPrefSize : nullptr);
    nsScrollbarFrame* scrollbar = do_QueryFrame(mHelper.mVScrollbarBox);
    scrollbar->SetScrollbarMediatorContent(mContent);
  }
  nscoord vScrollbarDesiredWidth =
      aAssumeVScroll ? vScrollbarPrefSize.width : 0;
-  nscoord vScrollbarMinHeight = aAssumeVScroll ? vScrollbarMinSize.height : 0;

-  nsSize hScrollbarMinSize(0, 0);
  nsSize hScrollbarPrefSize(0, 0);
  if (mHelper.mHScrollbarBox) {
    GetScrollbarMetrics(aState->mBoxState, mHelper.mHScrollbarBox,
-                        &hScrollbarMinSize,
+                        nullptr,
                        aAssumeHScroll ? &hScrollbarPrefSize : nullptr);
    nsScrollbarFrame* scrollbar = do_QueryFrame(mHelper.mHScrollbarBox);
    scrollbar->SetScrollbarMediatorContent(mContent);
  }
+
  nscoord hScrollbarDesiredHeight =
      aAssumeHScroll ? hScrollbarPrefSize.height : 0;
-  nscoord hScrollbarMinWidth = aAssumeHScroll ? hScrollbarMinSize.width : 0;

  // First, compute our inside-border size and scrollport size
  // XXXldb Can we depend more on ComputeSize here?
@ -446,10 +440,8 @@ bool nsHTMLScrollFrame::TryLayout(ScrollReflowInput* aState,
                       ? nsSize(0, 0)
                       : aKidMetrics->PhysicalSize();
  nsSize desiredInsideBorderSize;
-  desiredInsideBorderSize.width =
-      vScrollbarDesiredWidth + std::max(kidSize.width, hScrollbarMinWidth);
-  desiredInsideBorderSize.height =
-      hScrollbarDesiredHeight + std::max(kidSize.height, vScrollbarMinHeight);
+  desiredInsideBorderSize.width = vScrollbarDesiredWidth + kidSize.width;
+  desiredInsideBorderSize.height = hScrollbarDesiredHeight + kidSize.height;
  aState->mInsideBorderSize =
      ComputeInsideBorderSize(aState, desiredInsideBorderSize);

@ -507,9 +499,9 @@ bool nsHTMLScrollFrame::TryLayout(ScrollReflowInput* aState,
          aState->mHScrollbar == ShowScrollbar::Always ||
          scrolledRect.XMost() >= visualViewportSize.width + oneDevPixel ||
          scrolledRect.x <= -oneDevPixel;
-      if (scrollPortSize.width < hScrollbarMinSize.width)
-        wantHScrollbar = false;
-      if (wantHScrollbar != aAssumeHScroll) return false;
+      if (wantHScrollbar != aAssumeHScroll) {
+        return false;
+      }
    }

    // If the style is HIDDEN then we already know that aAssumeVScroll is false
@ -518,9 +510,9 @@ bool nsHTMLScrollFrame::TryLayout(ScrollReflowInput* aState,
          aState->mVScrollbar == ShowScrollbar::Always ||
          scrolledRect.YMost() >= visualViewportSize.height + oneDevPixel ||
          scrolledRect.y <= -oneDevPixel;
-      if (scrollPortSize.height < vScrollbarMinSize.height)
-        wantVScrollbar = false;
-      if (wantVScrollbar != aAssumeVScroll) return false;
+      if (wantVScrollbar != aAssumeVScroll) {
+        return false;
+      }
    }
  }

--- a/layout/xul/test/test_bug563416.html
+++ b/layout/xul/test/test_bug563416.html
@ -10,7 +10,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=563416
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=563416">Mozilla Bug 563416</a>
-<p id="display"><iframe id="test" srcdoc='<textarea style="box-sizing:content-box; -moz-appearance:none; height: 0px; padding: 0px;" cols="20" rows="10">hsldkjvmshlkkajskdlfksdjflskdjflskdjflskdjflskdjfddddddddd</textarea>'></iframe></p>
+<p id="display"><iframe id="test" srcdoc='<textarea style="box-sizing:content-box; overflow: hidden; -moz-appearance:none; height: 0px; padding: 0px;" cols="20" rows="10">hsldkjvmshlkkajskdlfksdjflskdjflskdjflskdjflskdjfddddddddd</textarea>'></iframe></p>
 <div id="content" style="display: none">
  
 </div>
--- a/testing/web-platform/meta/css/css-contain/contain-size-flexbox-002.html.ini
+++ b/testing/web-platform/meta/css/css-contain/contain-size-flexbox-002.html.ini
@ -1,4 +1,3 @@
 [contain-size-flexbox-002.html]
-  expected:
-    if (os == "android"): FAIL
+  expected: FAIL
  bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1560420
--- a/testing/web-platform/meta/css/css-grid/grid-model/grid-container-scrollbar-vertical-lr-001.html.ini
+++ b/testing/web-platform/meta/css/css-grid/grid-model/grid-container-scrollbar-vertical-lr-001.html.ini
@ -1,4 +0,0 @@
-[grid-container-scrollbar-vertical-lr-001.html]
-  expected:
-    if os == "mac": FAIL
-    if (os == 'linux' and os_version == '18.04'): FAIL # Bug 1601497
--- a/testing/web-platform/meta/css/css-grid/grid-model/grid-container-scrollbar-vertical-rl-001.html.ini
+++ b/testing/web-platform/meta/css/css-grid/grid-model/grid-container-scrollbar-vertical-rl-001.html.ini
@ -1,4 +0,0 @@
-[grid-container-scrollbar-vertical-rl-001.html]
-  expected:
-    if os == "mac": FAIL
-    if (os == 'linux' and os_version == '18.04'): FAIL # Bug 1601500
--- a/testing/web-platform/tests/css/css-overflow/scrollbar-empty-001.html
+++ b/testing/web-platform/tests/css/css-overflow/scrollbar-empty-001.html
@ -0,0 +1,12 @@
+<!doctype html>
+<title>overflow-y: scroll on an empty element doesn't cause it to gain height</title>
+<link rel="help" href="https://drafts.csswg.org/css-overflow/#propdef-overflow-y">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1365806">
+<link rel="match" href="/css/reference/blank.html">
+<style>
+  div {
+    background: red;
+    overflow-y: scroll;
+  }
+</style>
+<div></div>
--- a/testing/web-platform/tests/css/css-scrollbars/auto-scrollbar-inline-children.html
+++ b/testing/web-platform/tests/css/css-scrollbars/auto-scrollbar-inline-children.html
@ -9,7 +9,7 @@
  .container { width:100px; }
 </style>
 <p>There should be a blue rectangle below, and possibly a scrollbar (depending
-  on OS / browser), that shouldn not obscure any parts of the rectangle. The
+  on OS / browser), that should not obscure any parts of the rectangle. The
  word "FAIL" should not be seen.</p>
 <div class="container" style="overflow:auto; height:200px;">
  <div id="child" style="display:inline-block; box-sizing:border-box; width:100%; height:100%; border:10px solid blue;"></div>