From e02af14d81049efc05808eac51b5c7626479dfe3 Mon Sep 17 00:00:00 2001 From: Xidorn Quan Date: Wed, 10 Jun 2015 23:13:12 +1200 Subject: [PATCH] Bug 1161802 part 2 - Split nsGlobalWindow::SetFullScreenInternal into two parts, one part before the window resizing, the other after. r=smaug,dao,margaret This patch moves the "fullscreen" event from the original place to the second part, which indicates two other changes: 1. When the event is triggered, the value of fullScreen would have been toggled to the new value, which is different from before. The changes in browser/../browser-fullScreen.js and mobile/../browser.js are for this. 2. This event is no longer preventDefault-able, since it is triggered after the fullscreen change. This leads to the removal of the test and the only place which calls preventDefault on that event. That place is a workaround for bug 1079222. To address that problem, this patch fixes the intrinsic issue via stoping handling the fullscreen change once it finds we failed to change the state of the widget. --- browser/base/content/browser-fullScreen.js | 10 ++-- browser/base/content/sanitize.js | 17 ------- dom/base/nsGlobalWindow.cpp | 51 ++++++++++++++----- dom/base/nsGlobalWindow.h | 3 +- dom/base/nsPIDOMWindow.h | 16 +++++- dom/tests/mochitest/chrome/chrome.ini | 2 - .../chrome/fullscreen_preventdefault.xul | 27 ---------- .../chrome/test_fullscreen_preventdefault.xul | 37 -------------- mobile/android/chrome/content/browser.js | 2 +- xpfe/appshell/nsWebShellWindow.cpp | 14 ++++- xpfe/appshell/nsWebShellWindow.h | 1 + 11 files changed, 71 insertions(+), 109 deletions(-) delete mode 100644 dom/tests/mochitest/chrome/fullscreen_preventdefault.xul delete mode 100644 dom/tests/mochitest/chrome/test_fullscreen_preventdefault.xul diff --git a/browser/base/content/browser-fullScreen.js b/browser/base/content/browser-fullScreen.js index acbf87902ac..17a5af6b5e2 100644 --- a/browser/base/content/browser-fullScreen.js +++ b/browser/base/content/browser-fullScreen.js @@ -33,13 +33,9 @@ var FullScreen = { this.cleanup(); }, - toggle: function (event) { + toggle: function () { var enterFS = window.fullScreen; - // We get the fullscreen event _before_ the window transitions into or out of FS mode. - if (event && event.type == "fullscreen") - enterFS = !enterFS; - // Toggle the View:FullScreen command, which controls elements like the // fullscreen menuitem, and menubars. let fullscreenCommand = document.getElementById("View:FullScreen"); @@ -106,7 +102,7 @@ var FullScreen = { } break; case "fullscreen": - this.toggle(event); + this.toggle(); break; case "transitionend": if (event.propertyName == "opacity") @@ -192,7 +188,7 @@ var FullScreen = { }, cleanup: function () { - if (window.fullScreen) { + if (!window.fullScreen) { MousePosTracker.removeListener(this); document.removeEventListener("keypress", this._keyToggleCallback, false); document.removeEventListener("popupshown", this._setPopupOpen, false); diff --git a/browser/base/content/sanitize.js b/browser/base/content/sanitize.js index 99d207176af..6da116fd3b9 100644 --- a/browser/base/content/sanitize.js +++ b/browser/base/content/sanitize.js @@ -573,20 +573,6 @@ Sanitizer.prototype = { let features = "chrome,all,dialog=no," + this.privateStateForNewWindow; let newWindow = existingWindow.openDialog("chrome://browser/content/", "_blank", features, defaultArgs); -#ifdef XP_MACOSX - function onFullScreen(e) { - newWindow.removeEventListener("fullscreen", onFullScreen); - let docEl = newWindow.document.documentElement; - let sizemode = docEl.getAttribute("sizemode"); - if (!newWindow.fullScreen && sizemode == "fullscreen") { - docEl.setAttribute("sizemode", "normal"); - e.preventDefault(); - e.stopPropagation(); - return false; - } - } - newWindow.addEventListener("fullscreen", onFullScreen); -#endif // Window creation and destruction is asynchronous. We need to wait // until all existing windows are fully closed, and the new window is @@ -600,9 +586,6 @@ Sanitizer.prototype = { return; Services.obs.removeObserver(onWindowOpened, "browser-delayed-startup-finished"); -#ifdef XP_MACOSX - newWindow.removeEventListener("fullscreen", onFullScreen); -#endif newWindowOpened = true; // If we're the last thing to happen, invoke callback. if (numWindowsClosing == 0) { diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index 9e609e8e7cd..d3f54fcf523 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -6119,12 +6119,6 @@ nsGlobalWindow::SetFullScreenInternal(bool aFullScreen, bool aFullscreenMode, } } - // dispatch a "fullscreen" DOM event so that XUL apps can - // respond visually if we are kicked into full screen mode - if (!DispatchCustomEvent(NS_LITERAL_STRING("fullscreen"))) { - return NS_OK; - } - // Prevent chrome documents which are still loading from resizing // the window after we set fullscreen mode. nsCOMPtr treeOwnerAsWin = GetTreeOwnerWindow(); @@ -6152,9 +6146,43 @@ nsGlobalWindow::SetFullScreenInternal(bool aFullScreen, bool aFullscreenMode, widget->PrepareForDOMFullscreenTransition(); } widget->MakeFullScreen(aFullScreen, screen); + // The rest of code for switching fullscreen is in nsGlobalWindow:: + // FinishFullscreenChange() which will be called after sizemodechange + // event is dispatched. + return NS_OK; } } + FinishFullscreenChange(aFullScreen); + return NS_OK; +} + +/* virtual */ void +nsGlobalWindow::FinishFullscreenChange(bool aIsFullscreen) +{ + MOZ_ASSERT(IsOuterWindow()); + + if (aIsFullscreen != mFullScreen) { + NS_WARNING("Failed to toggle fullscreen state of the widget"); + // We failed to make the widget enter fullscreen. + // Stop further changes and restore the state. + if (!aIsFullscreen) { + mFullScreen = false; + mFullscreenMode = false; + } else { + MOZ_ASSERT_UNREACHABLE("Failed to exit fullscreen?"); + mFullScreen = true; + // We don't know how code can reach here. Not sure + // what value should be set for fullscreen mode. + mFullscreenMode = false; + } + return; + } + + // dispatch a "fullscreen" DOM event so that XUL apps can + // respond visually if we are kicked into full screen mode + DispatchCustomEvent(NS_LITERAL_STRING("fullscreen")); + if (!mFullScreen) { // Force exit from DOM full-screen mode. This is so that if we're in // DOM full-screen mode and the user exits full-screen mode with @@ -6166,23 +6194,20 @@ nsGlobalWindow::SetFullScreenInternal(bool aFullScreen, bool aFullscreenMode, if (!mWakeLock && mFullScreen) { nsRefPtr pmService = power::PowerManagerService::GetInstance(); - NS_ENSURE_TRUE(pmService, NS_OK); + if (!pmService) { + return; + } ErrorResult rv; mWakeLock = pmService->NewWakeLock(NS_LITERAL_STRING("DOM_Fullscreen"), this, rv); - if (rv.Failed()) { - return rv.StealNSResult(); - } - + NS_WARN_IF_FALSE(!rv.Failed(), "Failed to lock the wakelock"); } else if (mWakeLock && !mFullScreen) { ErrorResult rv; mWakeLock->Unlock(rv); NS_WARN_IF_FALSE(!rv.Failed(), "Failed to unlock the wakelock."); mWakeLock = nullptr; } - - return NS_OK; } bool diff --git a/dom/base/nsGlobalWindow.h b/dom/base/nsGlobalWindow.h index 274bd542dbb..15871c2c8ff 100644 --- a/dom/base/nsGlobalWindow.h +++ b/dom/base/nsGlobalWindow.h @@ -491,7 +491,8 @@ public: // Outer windows only. virtual nsresult SetFullScreenInternal(bool aIsFullscreen, bool aFullscreenMode, - mozilla::gfx::VRHMDInfo *aHMD = nullptr) override; + mozilla::gfx::VRHMDInfo *aHMD = nullptr) override final; + virtual void FinishFullscreenChange(bool aIsFullscreen) override final; bool FullScreen() const; // Inner windows only. diff --git a/dom/base/nsPIDOMWindow.h b/dom/base/nsPIDOMWindow.h index 939413ea591..df47f40d189 100644 --- a/dom/base/nsPIDOMWindow.h +++ b/dom/base/nsPIDOMWindow.h @@ -62,8 +62,8 @@ enum UIStateChangeType }; #define NS_PIDOMWINDOW_IID \ -{ 0x2485d4d7, 0xf7cb, 0x481e, \ - { 0x9c, 0x89, 0xb2, 0xa8, 0x12, 0x67, 0x7f, 0x97 } } +{ 0x0df7578f, 0x31d4, 0x4391, \ + { 0x98, 0xd4, 0xf3, 0x7d, 0x51, 0x8e, 0xa4, 0x37 } } class nsPIDOMWindow : public nsIDOMWindowInternal { @@ -475,6 +475,18 @@ public: virtual nsresult SetFullScreenInternal(bool aIsFullscreen, bool aFullscreenMode, mozilla::gfx::VRHMDInfo *aHMD = nullptr) = 0; + /** + * This function should be called when the fullscreen state is flipped. + * If no widget is involved the fullscreen change, this method is called + * by SetFullScreenInternal, otherwise, it is called when the widget + * finishes its change to or from fullscreen. + * + * @param aIsFullscreen indicates whether the widget is in fullscreen. + * + * Outer windows only. + */ + virtual void FinishFullscreenChange(bool aIsFullscreen) = 0; + /** * Call this to check whether some node (this window, its document, * or content in that document) has a mouseenter/leave event listener. diff --git a/dom/tests/mochitest/chrome/chrome.ini b/dom/tests/mochitest/chrome/chrome.ini index e09f43b9872..d43f22f32eb 100644 --- a/dom/tests/mochitest/chrome/chrome.ini +++ b/dom/tests/mochitest/chrome/chrome.ini @@ -15,7 +15,6 @@ support-files = focus_frameset.html focus_window2.xul fullscreen.xul - fullscreen_preventdefault.xul queryCaretRectUnix.html queryCaretRectWin.html selectAtPoint.html @@ -45,7 +44,6 @@ skip-if = buildapp == 'mulet' [test_fullscreen.xul] # disabled on linux for timeouts--bug-867745 skip-if = os == 'linux' -[test_fullscreen_preventdefault.xul] [test_geolocation.xul] [test_indexedSetter.html] [test_moving_nodeList.xul] diff --git a/dom/tests/mochitest/chrome/fullscreen_preventdefault.xul b/dom/tests/mochitest/chrome/fullscreen_preventdefault.xul deleted file mode 100644 index 658d6b2d51d..00000000000 --- a/dom/tests/mochitest/chrome/fullscreen_preventdefault.xul +++ /dev/null @@ -1,27 +0,0 @@ - - - - - - - - - - -