Bug 1276537 part 1 - Make resizable window on macOS always show full screen button. r=spohl
authorXidorn Quan <me@upsuper.org>
Fri, 15 May 2020 06:19:14 +0000
changeset 530232 7b2be27b7bb98f63cc72bb64871ca05a239ed782
parent 530231 5299f734099081ca91f2455457010a14cc47b42c
child 530233 6d7fac080dca7f3ec0fdd86c9e84834a7e68a9e3
push id116053
push usermozilla@upsuper.org
push dateFri, 15 May 2020 06:41:20 +0000
treeherderautoland@ea043dfff6a6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersspohl
bugs1276537
milestone78.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 1276537 part 1 - Make resizable window on macOS always show full screen button. r=spohl Differential Revision: https://phabricator.services.mozilla.com/D74833
layout/forms/test/test_bug665540.html
layout/style/test/chrome/test_display_mode.html
layout/style/test/chrome/test_display_mode_reflow.html
widget/cocoa/nsCocoaWindow.h
widget/cocoa/nsCocoaWindow.mm
widget/tests/window_bug522217.xhtml
--- a/layout/forms/test/test_bug665540.html
+++ b/layout/forms/test/test_bug665540.html
@@ -25,43 +25,65 @@ SimpleTest.waitForExplicitFinish();
 var win;
 var select;
 var optiona;
 var eventType = "mouseover";
 var timeoutID;
 var eventOffsetX = 2;
 var eventOffsetY = 2;
 
+let sizemodeChanged = false;
+let fullscreenChanged = false;
+let childHasFocused = false;
+
 function openFullscreenWindow() {
     win = window.docShell.rootTreeItem.domWindow
                 .openDialog("bug665540_window.xhtml", "_blank", "resizable=yes,chrome", window);
-    win.addEventListener("sizemodechange",
-                         function() {
-                           info("sizemodechange. windowState = " + win.windowState + " fullScreen = " + win.fullScreen);
-                         });
-    win.addEventListener("fullscreen",
-                         function() {
-                           info("fullscreen event. windowState = " + win.windowState + " fullScreen = " + win.fullScreen);
-                         });
     info("win.windowState = " + win.windowState);
     info("win.fullScreen = " + win.fullScreen);
 
+    // size mode and full screen may change asynchronously after child gets focused.
+    if (win.windowState == win.STATE_FULLSCREEN) {
+        sizemodeChanged = true;
+    } else {
+        win.addEventListener("sizemodechange", () => {
+            info("sizemodechange. windowState = " + win.windowState + " fullScreen = " + win.fullScreen);
+            sizemodeChanged = true;
+            tryStart();
+        }, { once: true });
+    }
+    if (win.fullScreen) {
+        fullscreenChanged = true;
+    } else {
+        win.addEventListener("fullscreen", () => {
+            info("fullscreen event. windowState = " + win.windowState + " fullScreen = " + win.fullScreen);
+            fullscreenChanged = true;
+            tryStart();
+        }, { once: true });
+    }
+
     // Close our window if the test times out so that it doesn't interfere
     // with later tests.
     timeoutID = setTimeout(function () {
         ok(false, "Test timed out.");
         // Provide some time for a screenshot
         setTimeout(finish, 1000);
     }, 20000);
 }
 
 function childFocused() {
-    ok(win.fullScreen, "window should be fullscreen");
-    is(win.windowState, win.STATE_FULLSCREEN,
-       "window state should be fullscreen");
+    info("child focused. windowState = " + win.windowState + " fullScreen = " + win.fullScreen);
+    childHasFocused = true;
+    tryStart();
+}
+
+function tryStart() {
+    if (!sizemodeChanged || !fullscreenChanged || !childHasFocused) {
+        return;
+    }
 
     // The select doesn't open if the mouse click is fired too soon
     // (on X11 at least).
     setTimeout(openSelect, 1000);
 }
 
 function openSelect() {
     select = win.document.getElementById("select");
--- a/layout/style/test/chrome/test_display_mode.html
+++ b/layout/style/test/chrome/test_display_mode.html
@@ -22,17 +22,17 @@ https://bugzilla.mozilla.org/show_bug.cg
               ["dom.security.featurePolicy.webidl.enabled", true],
             ],
           },
           r
         );
       });
       // Chrome test run tests in iframe, and fullscreen is disabled by default.
       // So run the test in a separate window.
-      window.open("display_mode.html", "display_mode", "width=500,height=500");
+      window.open("display_mode.html", "display_mode", "width=500,height=500,resizable");
     }
   </script>
 </head>
 <body onload="startTest();">
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1104916">Mozilla Bug 1104916</a>
 </div>
 <pre id="test">
 </pre>
--- a/layout/style/test/chrome/test_display_mode_reflow.html
+++ b/layout/style/test/chrome/test_display_mode_reflow.html
@@ -22,17 +22,17 @@ https://bugzilla.mozilla.org/show_bug.cg
               ["dom.security.featurePolicy.webidl.enabled", true],
             ],
           },
           r
         );
       });
       // Chrome test run tests in iframe, and fullscreen is disabled by default.
       // So run the test in a separate window.
-      window.open("display_mode_reflow.html", "display_mode_reflow", "width=500,height=500");
+      window.open("display_mode_reflow.html", "display_mode_reflow", "width=500,height=500,resizable");
     }
   </script>
 </head>
 <body onload="startTest();">
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1256084">Mozilla Bug 1256084</a>
 <p id="display"></p>
 <div id="content" style="display: none">
 </div>
--- a/widget/cocoa/nsCocoaWindow.h
+++ b/widget/cocoa/nsCocoaWindow.h
@@ -297,17 +297,16 @@ class nsCocoaWindow final : public nsBas
   virtual bool HasPendingInputEvent() override;
   virtual nsTransparencyMode GetTransparencyMode() override;
   virtual void SetTransparencyMode(nsTransparencyMode aMode) override;
   virtual void SetWindowShadowStyle(mozilla::StyleWindowShadow aStyle) override;
   virtual void SetWindowOpacity(float aOpacity) override;
   virtual void SetWindowTransform(const mozilla::gfx::Matrix& aTransform) override;
   virtual void SetWindowMouseTransparent(bool aIsTransparent) override;
   virtual void SetShowsToolbarButton(bool aShow) override;
-  virtual void SetShowsFullScreenButton(bool aShow) override;
   virtual void SetWindowAnimationType(WindowAnimationType aType) override;
   virtual void SetDrawsTitle(bool aDrawTitle) override;
   virtual void SetUseBrightTitlebarForeground(bool aBrightForeground) override;
   virtual nsresult SetNonClientMargins(LayoutDeviceIntMargin& aMargins) override;
   virtual void SetDrawsInTitlebar(bool aState) override;
   virtual void UpdateThemeGeometries(const nsTArray<ThemeGeometry>& aThemeGeometries) override;
   virtual nsresult SynthesizeNativeMouseEvent(LayoutDeviceIntPoint aPoint, uint32_t aNativeMessage,
                                               uint32_t aModifierFlags,
@@ -382,18 +381,16 @@ class nsCocoaWindow final : public nsBas
   bool mSheetNeedsShow;  // if this is a sheet, are we waiting to be shown?
                          // this is used for sibling sheet contention only
   bool mInFullScreenMode;
   bool mInFullScreenTransition;  // true from the request to enter/exit fullscreen
                                  // (MakeFullScreen() call) to EnteredFullScreen()
   bool mModal;
   bool mFakeModal;
 
-  // Only true on 10.7+ if SetShowsFullScreenButton(true) is called.
-  bool mSupportsNativeFullScreen;
   // Whether we are currently using native fullscreen. It could be false because
   // we are in the DOM fullscreen where we do not use the native fullscreen.
   bool mInNativeFullScreenMode;
 
   bool mIsAnimationSuppressed;
 
   bool mInReportMoveEvent;  // true if in a call to ReportMoveEvent().
   bool mInResize;           // true if in a call to DoResize().
--- a/widget/cocoa/nsCocoaWindow.mm
+++ b/widget/cocoa/nsCocoaWindow.mm
@@ -153,17 +153,16 @@ nsCocoaWindow::nsCocoaWindow()
       mBackingScaleFactor(0.0),
       mAnimationType(nsIWidget::eGenericWindowAnimation),
       mWindowMadeHere(false),
       mSheetNeedsShow(false),
       mInFullScreenMode(false),
       mInFullScreenTransition(false),
       mModal(false),
       mFakeModal(false),
-      mSupportsNativeFullScreen(false),
       mInNativeFullScreenMode(false),
       mIsAnimationSuppressed(false),
       mInReportMoveEvent(false),
       mInResize(false),
       mWindowTransformIsIdentity(true),
       mAlwaysOnTop(false),
       mAspectRatioLocked(false),
       mNumModalDescendents(0),
@@ -489,22 +488,25 @@ nsresult nsCocoaWindow::CreateNativeWind
     [mWindow setBackgroundColor:[NSColor clearColor]];
     [mWindow setOpaque:NO];
   } else {
     // Make sure that regular windows are opaque from the start, so that
     // nsChildView::WidgetTypeSupportsAcceleration returns true for them.
     [mWindow setOpaque:YES];
   }
 
+  NSWindowCollectionBehavior newBehavior = [mWindow collectionBehavior];
   if (mAlwaysOnTop) {
     [mWindow setLevel:NSFloatingWindowLevel];
-    NSWindowCollectionBehavior newBehavior = [mWindow collectionBehavior];
     newBehavior |= NSWindowCollectionBehaviorCanJoinAllSpaces;
-    [mWindow setCollectionBehavior:newBehavior];
   }
+  if ((features & NSResizableWindowMask)) {
+    newBehavior |= NSWindowCollectionBehaviorFullScreenPrimary;
+  }
+  [mWindow setCollectionBehavior:newBehavior];
 
   [mWindow setContentMinSize:NSMakeSize(60, 60)];
   [mWindow disableCursorRects];
 
   // Make the window use CoreAnimation from the start, so that we don't
   // switch from a non-CA window to a CA-window in the middle.
   [[mWindow contentView] setWantsLayer:YES];
 
@@ -1573,20 +1575,16 @@ void nsCocoaWindow::UpdateFullscreenStat
   DispatchSizeModeEvent();
   if (mWidgetListener && wasInFullscreen != aFullScreen) {
     mWidgetListener->FullscreenChanged(aFullScreen);
   }
 }
 
 inline bool nsCocoaWindow::ShouldToggleNativeFullscreen(bool aFullScreen,
                                                         bool aUseSystemTransition) {
-  if (!mSupportsNativeFullScreen) {
-    // If we cannot use native fullscreen, don't touch it.
-    return false;
-  }
   if (mInNativeFullScreenMode) {
     // If we are using native fullscreen, go ahead to exit it.
     return true;
   }
   if (!aUseSystemTransition) {
     // If we do not want the system fullscreen transition,
     // don't use the native fullscreen.
     return false;
@@ -1616,24 +1614,16 @@ nsresult nsCocoaWindow::DoMakeFullScreen
   // it gracefully - no need to ASSERT.
   if (mInFullScreenMode == aFullScreen) {
     return NS_OK;
   }
 
   mInFullScreenTransition = true;
 
   if (ShouldToggleNativeFullscreen(aFullScreen, aUseSystemTransition)) {
-    // If we're using native fullscreen mode and our native window is invisible,
-    // our attempt to go into fullscreen mode will fail with an assertion in
-    // system code, without [WindowDelegate windowDidFailToEnterFullScreen:]
-    // ever getting called.  To pre-empt this we bail here.  See bug 752294.
-    if (aFullScreen && ![mWindow isVisible]) {
-      EnteredFullScreen(false);
-      return NS_OK;
-    }
     MOZ_ASSERT(mInNativeFullScreenMode != aFullScreen,
                "We shouldn't have been in native fullscreen.");
     // Calling toggleFullScreen will result in windowDid(FailTo)?(Enter|Exit)FullScreen
     // to be called from the OS. We will call EnteredFullScreen from those methods,
     // where mInFullScreenMode will be set and a sizemode event will be dispatched.
     [mWindow toggleFullScreen:nil];
   } else {
     NSDisableScreenUpdates();
@@ -2302,51 +2292,16 @@ void nsCocoaWindow::SetWindowMouseTransp
 void nsCocoaWindow::SetShowsToolbarButton(bool aShow) {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
 
   if (mWindow) [mWindow setShowsToolbarButton:aShow];
 
   NS_OBJC_END_TRY_ABORT_BLOCK;
 }
 
-void nsCocoaWindow::SetShowsFullScreenButton(bool aShow) {
-  NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
-
-  if (!mWindow || ![mWindow respondsToSelector:@selector(toggleFullScreen:)] ||
-      mSupportsNativeFullScreen == aShow) {
-    return;
-  }
-
-  // If the window is currently in fullscreen mode, then we're going to
-  // transition out first, then set the collection behavior & toggle
-  // mSupportsNativeFullScreen, then transtion back into fullscreen mode. This
-  // prevents us from getting into a conflicting state with MakeFullScreen
-  // where mSupportsNativeFullScreen would lead us down the wrong path.
-  bool wasFullScreen = mInFullScreenMode;
-
-  if (wasFullScreen) {
-    MakeFullScreen(false);
-  }
-
-  NSWindowCollectionBehavior newBehavior = [mWindow collectionBehavior];
-  if (aShow) {
-    newBehavior |= NSWindowCollectionBehaviorFullScreenPrimary;
-  } else {
-    newBehavior &= ~NSWindowCollectionBehaviorFullScreenPrimary;
-  }
-  [mWindow setCollectionBehavior:newBehavior];
-  mSupportsNativeFullScreen = aShow;
-
-  if (wasFullScreen) {
-    MakeFullScreen(true);
-  }
-
-  NS_OBJC_END_TRY_ABORT_BLOCK;
-}
-
 void nsCocoaWindow::SetWindowAnimationType(nsIWidget::WindowAnimationType aType) {
   mAnimationType = aType;
 }
 
 void nsCocoaWindow::SetDrawsTitle(bool aDrawTitle) {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
 
   if (![mWindow drawsContentsIntoWindowFrame]) {
--- a/widget/tests/window_bug522217.xhtml
+++ b/widget/tests/window_bug522217.xhtml
@@ -22,50 +22,59 @@ function is(aLeft, aRight, aMessage)
   window.arguments[0].SimpleTest.is(aLeft, aRight, aMessage);
 }
 
 function isnot(aLeft, aRight, aMessage)
 {
   window.arguments[0].SimpleTest.isnot(aLeft, aRight, aMessage);
 }
 
-function executeSoon(aFct)
-{
-  window.arguments[0].SimpleTest.executeSoon(aFct);
+function executeSoon() {
+  return new Promise(resolve => {
+    window.arguments[0].SimpleTest.executeSoon(resolve);
+  });
+}
+
+function waitForEvent(obj, name) {
+  return new Promise(resolve => {
+    obj.addEventListener(name, resolve, { once: true });
+  });
 }
 
-function start() {
-  window.onfocus = function () {
-    window.onfocus = null;
-    var oldOuterWidth = window.outerWidth, oldOuterHeight = window.outerHeight;
-    var oldInnerWidth = window.innerWidth, oldInnerHeight = window.innerHeight;
-    document.documentElement.setAttribute("drawintitlebar", "true");
+async function start() {
+  await waitForEvent(window, "focus");
+  var oldOuterWidth = window.outerWidth, oldOuterHeight = window.outerHeight;
+  var oldInnerWidth = window.innerWidth, oldInnerHeight = window.innerHeight;
+  document.documentElement.setAttribute("drawintitlebar", "true");
+
+  await executeSoon();
+  is(window.outerWidth, oldOuterWidth, "drawintitlebar shouldn't change the window's outerWidth");
+  is(window.outerHeight, oldOuterHeight, "drawintitlebar shouldn't change the window's outerHeight");
+  is(window.innerWidth, oldOuterWidth, "if drawintitlebar is set, innerWidth and outerWidth should be the same");
+  is(window.innerHeight, oldOuterHeight, "if drawintitlebar is set, innerHeight and outerHeight should be the same");
 
-    executeSoon(function() {
-      is(window.outerWidth, oldOuterWidth, "drawintitlebar shouldn't change the window's outerWidth");
-      is(window.outerHeight, oldOuterHeight, "drawintitlebar shouldn't change the window's outerHeight");
-      is(window.innerWidth, oldOuterWidth, "if drawintitlebar is set, innerWidth and outerWidth should be the same");
-      is(window.innerHeight, oldOuterHeight, "if drawintitlebar is set, innerHeight and outerHeight should be the same");
-      window.fullScreen = true;
-      window.fullScreen = false;
-      is(window.outerWidth, oldOuterWidth, "wrong outerWidth after fullscreen mode");
-      is(window.outerHeight, oldOuterHeight, "wrong outerHeight after fullscreen mode");
-      is(window.innerWidth, oldOuterWidth, "wrong innerWidth after fullscreen mode");
-      is(window.innerHeight, oldOuterHeight, "wrong innerHeight after fullscreen mode");
-      document.documentElement.removeAttribute("drawintitlebar");
+  // Wait for going full screen and back.
+  let sizemodeChange = waitForEvent(window, "sizemodechange");
+  window.fullScreen = true;
+  await sizemodeChange;
+  sizemodeChange = waitForEvent(window, "sizemodechange");
+  window.fullScreen = false;
+  await sizemodeChange;
+  is(window.outerWidth, oldOuterWidth, "wrong outerWidth after fullscreen mode");
+  is(window.outerHeight, oldOuterHeight, "wrong outerHeight after fullscreen mode");
+  is(window.innerWidth, oldOuterWidth, "wrong innerWidth after fullscreen mode");
+  is(window.innerHeight, oldOuterHeight, "wrong innerHeight after fullscreen mode");
+  document.documentElement.removeAttribute("drawintitlebar");
 
-      executeSoon(function() {
-        is(window.outerWidth, oldOuterWidth, "wrong outerWidth after removing drawintitlebar");
-        is(window.outerHeight, oldOuterHeight, "wrong outerHeight after removing drawintitlebar");
-        is(window.innerWidth, oldInnerWidth, "wrong innerWidth after removing drawintitlebar");
-        is(window.innerHeight, oldInnerHeight, "wrong innerHeight after removing drawintitlebar");
-        window.arguments[0].SimpleTest.finish();
-        window.close();
-      });
-    });
-  }
+  await executeSoon();
+  is(window.outerWidth, oldOuterWidth, "wrong outerWidth after removing drawintitlebar");
+  is(window.outerHeight, oldOuterHeight, "wrong outerHeight after removing drawintitlebar");
+  is(window.innerWidth, oldInnerWidth, "wrong innerWidth after removing drawintitlebar");
+  is(window.innerHeight, oldInnerHeight, "wrong innerHeight after removing drawintitlebar");
+  window.arguments[0].SimpleTest.finish();
+  window.close();
 }
 
 
 ]]>
 </script>
 
 </window>