Bug 1609446 - Make default window-constraints always show the content. r=mats,mstange
☠☠ backed out by 96ff6ce7acbf ☠ ☠
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 15 Apr 2020 01:44:25 +0000
changeset 591154 887f1769a2c67bdf5796bc8948f41b3afa3cb3f6
parent 591153 ac8c5466869fbbea570797a6c517d67ffadee472
child 591155 b0e62a6a0bbd41e21ff6ad760209261ea42878ba
push id2335
push userffxbld-merge
push dateMon, 25 May 2020 13:47:24 +0000
treeherdermozilla-release@69ca1d06f46a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats, mstange
bugs1609446
milestone77.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1609446 - Make default window-constraints always show the content. r=mats,mstange This code is used to determine the sizes of the top-level windows. However the code doesn't cause quite desirable behavior (see the bug, and comment 15). This patch does two things: * Unifies the html / xul code-paths. This shouldn't change behavior (because GetXULMinSize returns the fixed min-* property if present anyways), but makes the patch a bit simpler. * Makes the min-width of the XUL window be the pref size instead of the min-size (for the cases where you have no explicit min-width). This looks a bit counter intuitive, but it's the only way to guarantee that the content will be shown. This matches the sizing algorithm that dialogs use by default (via calling window.sizeToContent()), while allowing to undersize the window via a fixed min-width property. This in turn makes sizeToContent() work "by default" on XUL windows, avoiding having to make JS listen to everything that possibly could change the layout of the document (like resolution changes). Differential Revision: https://phabricator.services.mozilla.com/D70209
devtools/client/framework/toolbox-window.xhtml
gfx/src/nsSize.h
layout/base/Units.h
layout/base/nsDocumentViewer.cpp
layout/generic/nsContainerFrame.cpp
toolkit/content/tests/chrome/window_screenPosSize.xhtml
--- a/devtools/client/framework/toolbox-window.xhtml
+++ b/devtools/client/framework/toolbox-window.xhtml
@@ -1,17 +1,23 @@
 <?xml version="1.0" encoding="utf-8"?>
 <!-- This Source Code Form is subject to the terms of the Mozilla Public
    - License, v. 2.0. If a copy of the MPL was not distributed with this
    - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
 
 <?xml-stylesheet href="chrome://global/skin/global.css" type="text/css"?>
 
-<!-- minwidth=50 is sum width of chevron and meatball menu button. -->
+<!--
+  min-width: min-content makes us have the min size of our
+  browser element, which is set explicitly via css in
+  toolbox-host-manager.js. It could be set on the window
+  instead and then it wouldn't be needed.
+-->
 <window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
         id="devtools-toolbox-window"
         macanimationtype="document"
         fullscreenbutton="true"
         windowtype="devtools:toolbox"
         width="900" height="320"
+        style="min-width: min-content"
         persist="screenX screenY width height sizemode">
   <tooltip id="aHTMLTooltip" page="true"/>
 </window>
--- a/gfx/src/nsSize.h
+++ b/gfx/src/nsSize.h
@@ -21,16 +21,20 @@ struct nsSize : public mozilla::gfx::Bas
 
   nsSize() : Super() {}
   nsSize(nscoord aWidth, nscoord aHeight) : Super(aWidth, aHeight) {}
 
   inline mozilla::gfx::IntSize ScaleToNearestPixels(
       float aXScale, float aYScale, nscoord aAppUnitsPerPixel) const;
   inline mozilla::gfx::IntSize ToNearestPixels(nscoord aAppUnitsPerPixel) const;
 
+  inline mozilla::gfx::IntSize ScaleToOutsidePixels(
+      float aXScale, float aYScale, nscoord aAppUnitsPerPixel) const;
+  inline mozilla::gfx::IntSize ToOutsidePixels(nscoord aAppUnitsPerPixel) const;
+
   /**
    * Return this size scaled to a different appunits per pixel (APP) ratio.
    * @param aFromAPP the APP to scale from
    * @param aToAPP the APP to scale to
    */
   [[nodiscard]] inline nsSize ScaleToOtherAppUnits(int32_t aFromAPP,
                                                    int32_t aToAPP) const;
 };
@@ -39,21 +43,34 @@ inline mozilla::gfx::IntSize nsSize::Sca
     float aXScale, float aYScale, nscoord aAppUnitsPerPixel) const {
   return mozilla::gfx::IntSize(
       NSToIntRoundUp(NSAppUnitsToDoublePixels(width, aAppUnitsPerPixel) *
                      aXScale),
       NSToIntRoundUp(NSAppUnitsToDoublePixels(height, aAppUnitsPerPixel) *
                      aYScale));
 }
 
+inline mozilla::gfx::IntSize nsSize::ScaleToOutsidePixels(
+    float aXScale, float aYScale, nscoord aAppUnitsPerPixel) const {
+  return mozilla::gfx::IntSize(
+      NSToIntCeil(NSAppUnitsToDoublePixels(width, aAppUnitsPerPixel) * aXScale),
+      NSToIntCeil(NSAppUnitsToDoublePixels(height, aAppUnitsPerPixel) *
+                  aYScale));
+}
+
 inline mozilla::gfx::IntSize nsSize::ToNearestPixels(
     nscoord aAppUnitsPerPixel) const {
   return ScaleToNearestPixels(1.0f, 1.0f, aAppUnitsPerPixel);
 }
 
+inline mozilla::gfx::IntSize nsSize::ToOutsidePixels(
+    nscoord aAppUnitsPerPixel) const {
+  return ScaleToOutsidePixels(1.0f, 1.0f, aAppUnitsPerPixel);
+}
+
 inline nsSize nsSize::ScaleToOtherAppUnits(int32_t aFromAPP,
                                            int32_t aToAPP) const {
   if (aFromAPP != aToAPP) {
     nsSize size;
     size.width = NSToCoordRound(NSCoordScale(width, aFromAPP, aToAPP));
     size.height = NSToCoordRound(NSCoordScale(height, aFromAPP, aToAPP));
     return size;
   }
--- a/layout/base/Units.h
+++ b/layout/base/Units.h
@@ -424,16 +424,22 @@ struct LayoutDevicePixel {
   }
 
   static LayoutDeviceIntRect FromAppUnitsToOutside(
       const nsRect& aRect, nscoord aAppUnitsPerDevPixel) {
     return LayoutDeviceIntRect::FromUnknownRect(
         aRect.ToOutsidePixels(aAppUnitsPerDevPixel));
   }
 
+  static LayoutDeviceIntSize FromAppUnitsToOutside(
+      const nsSize& aSize, nscoord aAppUnitsPerDevPixel) {
+    return LayoutDeviceIntSize::FromUnknownSize(
+        aSize.ToOutsidePixels(aAppUnitsPerDevPixel));
+  }
+
   static LayoutDeviceIntSize FromAppUnitsRounded(const nsSize& aSize,
                                                  nscoord aAppUnitsPerDevPixel) {
     return LayoutDeviceIntSize(
         NSAppUnitsToIntPixels(aSize.width, aAppUnitsPerDevPixel),
         NSAppUnitsToIntPixels(aSize.height, aAppUnitsPerDevPixel));
   }
 
   static nsPoint ToAppUnits(const LayoutDeviceIntPoint& aPoint,
--- a/layout/base/nsDocumentViewer.cpp
+++ b/layout/base/nsDocumentViewer.cpp
@@ -3131,18 +3131,20 @@ nsresult nsDocumentViewer::GetContentSiz
   // Protect against bogus returns here
   nsRect shellArea = presContext->GetVisibleArea();
   NS_ENSURE_TRUE(shellArea.width != NS_UNCONSTRAINEDSIZE &&
                      shellArea.height != NS_UNCONSTRAINEDSIZE,
                  NS_ERROR_FAILURE);
 
   // Ceil instead of rounding here, so we can actually guarantee showing all the
   // content.
-  *aWidth = std::ceil(presContext->AppUnitsToFloatDevPixels(shellArea.width));
-  *aHeight = std::ceil(presContext->AppUnitsToFloatDevPixels(shellArea.height));
+  auto devOuterSize = LayoutDeviceIntSize::FromAppUnitsToOutside(
+      shellArea, presContext->AppUnitsPerDevPixel());
+  *aWidth = devOuterSize.width;
+  *aHeight = devOuterSize.height;
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDocumentViewer::GetContentSize(int32_t* aWidth, int32_t* aHeight) {
   NS_ENSURE_TRUE(mContainer, NS_ERROR_NOT_AVAILABLE);
 
--- a/layout/generic/nsContainerFrame.cpp
+++ b/layout/generic/nsContainerFrame.cpp
@@ -546,37 +546,43 @@ static bool IsTopLevelWidget(nsIWidget* 
   return windowType == eWindowType_toplevel ||
          windowType == eWindowType_dialog || windowType == eWindowType_popup ||
          windowType == eWindowType_sheet;
 }
 
 void nsContainerFrame::SyncWindowProperties(nsPresContext* aPresContext,
                                             nsIFrame* aFrame, nsView* aView,
                                             gfxContext* aRC, uint32_t aFlags) {
-#ifdef MOZ_XUL
-  if (!aView || !nsCSSRendering::IsCanvasFrame(aFrame) || !aView->HasWidget())
+  if (!aView || !nsCSSRendering::IsCanvasFrame(aFrame) || !aView->HasWidget()) {
     return;
+  }
 
   nsCOMPtr<nsIWidget> windowWidget =
       GetPresContextContainerWidget(aPresContext);
-  if (!windowWidget || !IsTopLevelWidget(windowWidget)) return;
+  if (!windowWidget || !IsTopLevelWidget(windowWidget)) {
+    return;
+  }
 
   nsViewManager* vm = aView->GetViewManager();
   nsView* rootView = vm->GetRootView();
 
-  if (aView != rootView) return;
+  if (aView != rootView) {
+    return;
+  }
 
   Element* rootElement = aPresContext->Document()->GetRootElement();
   if (!rootElement) {
     return;
   }
 
   nsIFrame* rootFrame =
       aPresContext->PresShell()->FrameConstructor()->GetRootElementStyleFrame();
-  if (!rootFrame) return;
+  if (!rootFrame) {
+    return;
+  }
 
   if (aFlags & SET_ASYNC) {
     aView->SetNeedsWindowPropertiesSync();
     return;
   }
 
   RefPtr<nsPresContext> kungFuDeathGrip(aPresContext);
   AutoWeakFrame weak(rootFrame);
@@ -595,66 +601,80 @@ void nsContainerFrame::SyncWindowPropert
     nsTransparencyMode mode =
         nsLayoutUtils::GetFrameTransparency(aFrame, rootFrame);
     StyleWindowShadow shadow = rootFrame->StyleUIReset()->mWindowShadow;
     nsCOMPtr<nsIWidget> viewWidget = aView->GetWidget();
     viewWidget->SetTransparencyMode(mode);
     windowWidget->SetWindowShadowStyle(shadow);
   }
 
-  if (!aRC) return;
+  if (!aRC) {
+    return;
+  }
 
   if (!weak.IsAlive()) {
     return;
   }
 
   nsSize minSize(0, 0);
   nsSize maxSize(NS_UNCONSTRAINEDSIZE, NS_UNCONSTRAINEDSIZE);
-  if (rootElement->IsXULElement()) {
-    nsBoxLayoutState aState(aPresContext, aRC);
-    minSize = rootFrame->GetXULMinSize(aState);
-    maxSize = rootFrame->GetXULMaxSize(aState);
-  } else {
-    auto* pos = rootFrame->StylePosition();
-    if (pos->mMinWidth.ConvertsToLength()) {
-      minSize.width = pos->mMinWidth.ToLength();
+
+  const auto& pos = *rootFrame->StylePosition();
+#ifdef MOZ_XUL
+  if (rootFrame->IsXULBoxFrame()) {
+    nsBoxLayoutState xulState(aPresContext, aRC);
+    minSize = rootFrame->GetXULMinSize(xulState);
+    const bool isMinContent =
+        pos.mMinWidth.IsExtremumLength() &&
+        pos.mMinWidth.AsExtremumLength() == StyleExtremumLength::MinContent;
+    // By default we behave with min-width: max-content, to ensure that all
+    // content is shown (i.e., the GetXULPrefSize call below).
+    //
+    // It can be overridden to min-content (i.e., leave GetXULMinSize()) and
+    // a fixed width (handled below).
+    if (!isMinContent && !pos.mMinWidth.ConvertsToLength()) {
+      minSize.width = rootFrame->GetXULPrefSize(xulState).width;
     }
-    if (pos->mMinHeight.ConvertsToLength()) {
-      minSize.height = pos->mMinHeight.ToLength();
-    }
-    if (pos->mMaxWidth.ConvertsToLength()) {
-      maxSize.width = pos->mMaxWidth.ToLength();
-    }
-    if (pos->mMaxHeight.ConvertsToLength()) {
-      maxSize.height = pos->mMaxHeight.ToLength();
-    }
+    maxSize = rootFrame->GetXULMaxSize(xulState);
+  }
+#endif
+
+  if (pos.mMinWidth.ConvertsToLength()) {
+    minSize.width = pos.mMinWidth.ToLength();
   }
+  if (pos.mMinHeight.ConvertsToLength()) {
+    minSize.height = pos.mMinHeight.ToLength();
+  }
+  if (pos.mMaxWidth.ConvertsToLength()) {
+    maxSize.width = pos.mMaxWidth.ToLength();
+  }
+  if (pos.mMaxHeight.ConvertsToLength()) {
+    maxSize.height = pos.mMaxHeight.ToLength();
+  }
+
   SetSizeConstraints(aPresContext, windowWidget, minSize, maxSize);
-#endif
 }
 
 void nsContainerFrame::SetSizeConstraints(nsPresContext* aPresContext,
                                           nsIWidget* aWidget,
                                           const nsSize& aMinSize,
                                           const nsSize& aMaxSize) {
-  LayoutDeviceIntSize devMinSize(
-      aPresContext->AppUnitsToDevPixels(aMinSize.width),
-      aPresContext->AppUnitsToDevPixels(aMinSize.height));
+  auto devMinSize = LayoutDeviceIntSize::FromAppUnitsToOutside(
+      aMinSize, aPresContext->AppUnitsPerDevPixel());
   LayoutDeviceIntSize devMaxSize(
       aMaxSize.width == NS_UNCONSTRAINEDSIZE
           ? NS_MAXSIZE
           : aPresContext->AppUnitsToDevPixels(aMaxSize.width),
       aMaxSize.height == NS_UNCONSTRAINEDSIZE
           ? NS_MAXSIZE
           : aPresContext->AppUnitsToDevPixels(aMaxSize.height));
 
   // MinSize has a priority over MaxSize
-  if (devMinSize.width > devMaxSize.width) devMaxSize.width = devMinSize.width;
-  if (devMinSize.height > devMaxSize.height)
-    devMaxSize.height = devMinSize.height;
+  devMaxSize.width = std::max(devMinSize.width, devMaxSize.width);
+  devMaxSize.height = std::max(devMinSize.height, devMaxSize.height);
 
   widget::SizeConstraints constraints(devMinSize, devMaxSize);
 
   // The sizes are in inner window sizes, so convert them into outer window
   // sizes. Use a size of (200, 200) as only the difference between the inner
   // and outer size is needed.
   LayoutDeviceIntSize windowSize =
       aWidget->ClientToWindowSize(LayoutDeviceIntSize(200, 200));
--- a/toolkit/content/tests/chrome/window_screenPosSize.xhtml
+++ b/toolkit/content/tests/chrome/window_screenPosSize.xhtml
@@ -3,15 +3,16 @@
 <?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css" type="text/css"?>
 
 <window title="Window Open Test"
         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
         screenX="80"
         screenY="80"
         height="300"
         width="300"
+        style="min-width: 0"
         persist="screenX screenY height width">
 
 <body xmlns="http://www.w3.org/1999/xhtml">
 
 </body>
 
 </window>