Bug 1444525: Persist XUL window size attributes as a window size, not client size. r?bz draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sat, 10 Mar 2018 02:42:57 +0100
changeset 765783 c7ddfe43244bb903dda762ba8276f285a88264fe
parent 765782 4114a9737082370e6388629eaa7b39f74ae60622
child 765903 333202648d1dd18d612c20af7c689aa9e9f88057
push id102161
push userbmo:emilio@crisal.io
push dateSat, 10 Mar 2018 07:39:34 +0000
reviewersbz
bugs1444525
milestone60.0a1
Bug 1444525: Persist XUL window size attributes as a window size, not client size. r?bz MozReview-Commit-ID: 7U5tWxTaxGi
dom/xul/XULDocument.cpp
xpfe/appshell/nsIXULWindow.idl
xpfe/appshell/nsXULWindow.cpp
--- a/dom/xul/XULDocument.cpp
+++ b/dom/xul/XULDocument.cpp
@@ -1156,16 +1156,51 @@ XULDocument::Persist(const nsAString& aI
         }
 
         nameSpaceID = kNameSpaceID_None;
     }
 
     aRv = Persist(element, nameSpaceID, tag);
 }
 
+enum class ConversionDirection {
+    InnerToOuter,
+    OuterToInner,
+};
+
+static void
+ConvertWindowSize(nsIXULWindow* aWin,
+                  nsAtom* aAttr,
+                  ConversionDirection aDirection,
+                  nsAString& aInOutString)
+{
+    MOZ_ASSERT(aWin);
+    MOZ_ASSERT(aAttr == nsGkAtoms::width || aAttr == nsGkAtoms::height);
+
+    nsresult rv;
+    int32_t size = aInOutString.ToInteger(&rv);
+    if (NS_FAILED(rv)) {
+        return;
+    }
+
+    int32_t sizeDiff = aAttr == nsGkAtoms::width
+        ? aWin->GetOuterToInnerWidthDifferenceInCSSPixels()
+        : aWin->GetOuterToInnerHeightDifferenceInCSSPixels();
+
+    if (!sizeDiff) {
+        return;
+    }
+
+    int32_t multiplier =
+        aDirection == ConversionDirection::InnerToOuter ? 1 : - 1;
+
+    aInOutString.Assign(
+      NS_ConvertASCIItoUTF16(nsPrintfCString("%d", size + multiplier * sizeDiff)));
+}
+
 nsresult
 XULDocument::Persist(Element* aElement, int32_t aNameSpaceID,
                      nsAtom* aAttribute)
 {
     // For non-chrome documents, persistance is simply broken
     if (!nsContentUtils::IsSystemPrincipal(NodePrincipal()))
         return NS_ERROR_NOT_AVAILABLE;
 
@@ -1194,19 +1229,31 @@ XULDocument::Persist(Element* aElement, 
     bool hasAttr;
     rv = mLocalStore->HasValue(uri, id, attrstr, &hasAttr);
     if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
     }
 
     if (hasAttr && valuestr.IsEmpty()) {
         return mLocalStore->RemoveValue(uri, id, attrstr);
-    } else {
-        return mLocalStore->SetValue(uri, id, attrstr, valuestr);
     }
+
+    // Make sure we store the <window> attributes as outer window size, see
+    // bug 1444525 & co.
+    if (aElement->IsXULElement(nsGkAtoms::window) &&
+        (aAttribute == nsGkAtoms::width || aAttribute == nsGkAtoms::height)) {
+        if (nsCOMPtr<nsIXULWindow> win = GetXULWindowIfToplevelChrome()) {
+            ConvertWindowSize(win,
+                              aAttribute,
+                              ConversionDirection::InnerToOuter,
+                              valuestr);
+        }
+    }
+
+    return mLocalStore->SetValue(uri, id, attrstr, valuestr);
 }
 
 
 nsresult
 XULDocument::GetViewportSize(int32_t* aWidth,
                              int32_t* aHeight)
 {
     *aWidth = *aHeight = 0;
@@ -1744,17 +1791,16 @@ XULDocument::ApplyPersistentAttributesIn
         if (NS_WARN_IF(NS_FAILED(rv))) {
             return rv;
         }
     }
 
     return NS_OK;
 }
 
-
 nsresult
 XULDocument::ApplyPersistentAttributesToElements(const nsAString &aID,
                                                  nsCOMArray<Element>& aElements)
 {
     nsAutoCString utf8uri;
     nsresult rv = mDocumentURI->GetSpec(utf8uri);
     if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
@@ -1785,23 +1831,39 @@ XULDocument::ApplyPersistentAttributesTo
         }
 
         RefPtr<nsAtom> attr = NS_Atomize(attrstr);
         if (NS_WARN_IF(!attr)) {
             return NS_ERROR_OUT_OF_MEMORY;
         }
 
         uint32_t cnt = aElements.Count();
-
         for (int32_t i = int32_t(cnt) - 1; i >= 0; --i) {
             RefPtr<Element> element = aElements.SafeObjectAt(i);
             if (!element) {
                  continue;
             }
 
+            // Scale attributes from window size to client size for top-level
+            // windows, see bug 1444525 & co.
+            if (element->IsXULElement(nsGkAtoms::window) &&
+                (attr == nsGkAtoms::width || attr == nsGkAtoms::height)) {
+                if (nsCOMPtr<nsIXULWindow> win = GetXULWindowIfToplevelChrome()) {
+                    nsAutoString maybeScaledValue = value;
+                    ConvertWindowSize(win,
+                                      attr,
+                                      ConversionDirection::OuterToInner,
+                                      maybeScaledValue);
+                    Unused <<
+                        element->SetAttr(kNameSpaceID_None, attr, maybeScaledValue, true);
+
+                    continue;
+                }
+            }
+
             Unused << element->SetAttr(kNameSpaceID_None, attr, value, true);
         }
     }
 
     return NS_OK;
 }
 
 void
--- a/xpfe/appshell/nsIXULWindow.idl
+++ b/xpfe/appshell/nsIXULWindow.idl
@@ -62,16 +62,23 @@ interface nsIXULWindow : nsISupports
 
   /**
    * Tell this window that it has picked up a child XUL window
    * @param aChild the child window being added
    */
   void addChildWindow(in nsIXULWindow aChild);
 
   /**
+   * Returns the difference between the inner window size (client size) and the
+   * outer window size, in CSS pixels.
+   */
+  [noscript,notxpcom] uint32_t getOuterToInnerHeightDifferenceInCSSPixels();
+  [noscript,notxpcom] uint32_t getOuterToInnerWidthDifferenceInCSSPixels();
+
+  /**
    * Tell this window that it has lost a child XUL window
    * @param aChild the child window being removed
    */
   void removeChildWindow(in nsIXULWindow aChild);
 
   /**
    * Move the window to a centered position.
    * @param aRelative If not null, the window relative to which the window is
--- a/xpfe/appshell/nsXULWindow.cpp
+++ b/xpfe/appshell/nsXULWindow.cpp
@@ -334,16 +334,49 @@ nsXULWindow::TabParentRemoved(nsITabPare
 NS_IMETHODIMP
 nsXULWindow::GetPrimaryTabParent(nsITabParent** aTab)
 {
   nsCOMPtr<nsITabParent> tab = mPrimaryTabParent;
   tab.forget(aTab);
   return NS_OK;
 }
 
+static LayoutDeviceIntSize
+GetOuterToInnerSizeDifference(nsIWidget* aWindow)
+{
+  if (!aWindow) {
+    return LayoutDeviceIntSize();
+  }
+  LayoutDeviceIntSize baseSize(200, 200);
+  LayoutDeviceIntSize windowSize = aWindow->ClientToWindowSize(baseSize);
+  return windowSize - baseSize;
+}
+
+static CSSIntSize
+GetOuterToInnerSizeDifferenceInCSSPixels(nsIWidget* aWindow)
+{
+  if (!aWindow) {
+    return { };
+  }
+  LayoutDeviceIntSize devPixelSize = GetOuterToInnerSizeDifference(aWindow);
+  return RoundedToInt(devPixelSize / aWindow->GetDefaultScale());
+}
+
+uint32_t
+nsXULWindow::GetOuterToInnerHeightDifferenceInCSSPixels()
+{
+  return GetOuterToInnerSizeDifferenceInCSSPixels(mWindow).height;
+}
+
+uint32_t
+nsXULWindow::GetOuterToInnerWidthDifferenceInCSSPixels()
+{
+  return GetOuterToInnerSizeDifferenceInCSSPixels(mWindow).width;
+}
+
 nsTArray<RefPtr<mozilla::LiveResizeListener>>
 nsXULWindow::GetLiveResizeListeners()
 {
   nsTArray<RefPtr<mozilla::LiveResizeListener>> listeners;
   if (mPrimaryTabParent) {
     TabParent* parent = static_cast<TabParent*>(mPrimaryTabParent.get());
     listeners.AppendElement(parent);
   }
@@ -1060,27 +1093,16 @@ NS_IMETHODIMP nsXULWindow::ForceRoundedD
   SetPrimaryContentSize(targetContentWidth, targetContentHeight);
 
   mIgnoreXULSize = true;
   mIgnoreXULSizeMode = true;
 
   return NS_OK;
 }
 
-static LayoutDeviceIntSize
-GetWindowOuterInnerDiff(nsIWidget* aWindow)
-{
-  if (!aWindow) {
-    return LayoutDeviceIntSize();
-  }
-  LayoutDeviceIntSize baseSize(200, 200);
-  LayoutDeviceIntSize windowSize = aWindow->ClientToWindowSize(baseSize);
-  return windowSize - baseSize;
-}
-
 void nsXULWindow::OnChromeLoaded()
 {
   nsresult rv = EnsureContentTreeOwner();
 
   if (NS_SUCCEEDED(rv)) {
     mChromeLoaded = true;
     ApplyChromeFlags();
     SyncAttributesToWidget();
@@ -1594,20 +1616,20 @@ NS_IMETHODIMP nsXULWindow::SavePersisten
     if (NS_SUCCEEDED(parent->GetPosition(&parentX, &parentY))) {
       rect.MoveBy(-parentX, -parentY);
     }
   }
 
   char                        sizeBuf[10];
   nsAutoString                sizeString;
   nsAutoString                windowElementId;
-  RefPtr<dom::XULDocument>    ownerXULDoc;
 
   // fetch docShellElement's ID and XUL owner document
-  ownerXULDoc = docShellElement->OwnerDoc()->AsXULDocument();
+  RefPtr<dom::XULDocument> ownerXULDoc =
+    docShellElement->OwnerDoc()->AsXULDocument();
   if (docShellElement->IsXULElement()) {
     docShellElement->GetId(windowElementId);
   }
 
   bool shouldPersist = !isFullscreen && ownerXULDoc;
   ErrorResult rv;
   // (only for size elements which are persisted)
   if ((mPersistentAttributesDirty & PAD_POSITION) && gotRestoredBounds) {
@@ -1627,30 +1649,28 @@ NS_IMETHODIMP nsXULWindow::SavePersisten
       if (shouldPersist) {
         IgnoredErrorResult err;
         ownerXULDoc->Persist(windowElementId, SCREENY_ATTRIBUTE, err);
       }
     }
   }
 
   if ((mPersistentAttributesDirty & PAD_SIZE) && gotRestoredBounds) {
-    LayoutDeviceIntSize winDiff = GetWindowOuterInnerDiff(mWindow);
+    LayoutDeviceIntRect innerRect = rect - GetOuterToInnerSizeDifference(mWindow);
     if (persistString.Find("width") >= 0) {
-      auto width = rect.Width() - winDiff.width;
-      SprintfLiteral(sizeBuf, "%d", NSToIntRound(width / sizeScale.scale));
+      SprintfLiteral(sizeBuf, "%d", NSToIntRound(innerRect.Width() / sizeScale.scale));
       CopyASCIItoUTF16(sizeBuf, sizeString);
       docShellElement->SetAttribute(WIDTH_ATTRIBUTE, sizeString, rv);
       if (shouldPersist) {
         IgnoredErrorResult err;
         ownerXULDoc->Persist(windowElementId, WIDTH_ATTRIBUTE, err);
       }
     }
     if (persistString.Find("height") >= 0) {
-      auto height = rect.Height() - winDiff.height;
-      SprintfLiteral(sizeBuf, "%d", NSToIntRound(height / sizeScale.scale));
+      SprintfLiteral(sizeBuf, "%d", NSToIntRound(innerRect.Height() / sizeScale.scale));
       CopyASCIItoUTF16(sizeBuf, sizeString);
       docShellElement->SetAttribute(HEIGHT_ATTRIBUTE, sizeString, rv);
       if (shouldPersist) {
         IgnoredErrorResult err;
         ownerXULDoc->Persist(windowElementId, HEIGHT_ATTRIBUTE, err);
       }
     }
   }
@@ -2249,20 +2269,17 @@ void
 nsXULWindow::SizeShell()
 {
   int32_t specWidth = -1, specHeight = -1;
   bool gotSize = false;
   bool isContent = false;
 
   GetHasPrimaryContent(&isContent);
 
-  CSSIntSize windowDiff = mWindow
-    ? RoundedToInt(GetWindowOuterInnerDiff(mWindow) /
-                   mWindow->GetDefaultScale())
-    : CSSIntSize();
+  CSSIntSize windowDiff = GetOuterToInnerSizeDifferenceInCSSPixels(mWindow);
 
   // If this window has a primary content and fingerprinting resistance is
   // enabled, we enforce this window to rounded dimensions.
   if (isContent && nsContentUtils::ShouldResistFingerprinting()) {
     ForceRoundedDimensions();
   } else if (!mIgnoreXULSize) {
     gotSize = LoadSizeFromXUL(specWidth, specHeight);
     specWidth += windowDiff.width;