Bug 1135261 - return new window from window.open in desktop runtime; r=marco,smaug,junior,wesj
☠☠ backed out by 98c2f8a27530 ☠ ☠
authorMyk Melez <myk@mozilla.org>
Tue, 21 Jul 2015 16:52:57 -0700
changeset 254030 35c6e7e9c205be0656c050909aaa3b42fa064c4f
parent 254029 564e2565cf837b01ad8ca5e899472f2a335ca121
child 254031 6ad92b48085b5ec5532f14e0b9e3c5eb15ad9fa8
push id62629
push usermyk@mozilla.com
push dateWed, 22 Jul 2015 02:16:10 +0000
treeherdermozilla-inbound@35c6e7e9c205 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarco, smaug, junior, wesj
bugs1135261
milestone42.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 1135261 - return new window from window.open in desktop runtime; r=marco,smaug,junior,wesj
dom/apps/tests/file_test_widget.js
dom/browser-element/BrowserElementParent.cpp
webapprt/content/webapp.js
webapprt/test/chrome/browser_window-open.js
webapprt/test/chrome/window-open.html
--- a/dom/apps/tests/file_test_widget.js
+++ b/dom/apps/tests/file_test_widget.js
@@ -153,17 +153,19 @@ function loadFrameScript(mm) {
   \
   var request = content.window.navigator.mozApps.getSelf(); \
   request.onsuccess = function() { \
   var widget = request.result; \
   ok(widget,"Should be a widget"); \
   checkWidget(widget); \
 }; \
   request.onerror = onError; \
-  content.window.open("about:blank"); /*test mozbrowseropenwindow*/ \
+  var win = content.window.open("about:blank"); /*test mozbrowseropenwindow*/ \
+  /*Close new window to avoid mochitest "unable to restore focus" failures.*/ \
+  win.close(); \
   content.window.scrollTo(4000, 4000); /*test mozbrowser(async)scroll*/ \
   ';
   mm.loadFrameScript(script, /* allowDelayedLoad = */ false);
 }
 
 var tests = [
   // Permissions
   function() {
--- a/dom/browser-element/BrowserElementParent.cpp
+++ b/dom/browser-element/BrowserElementParent.cpp
@@ -118,17 +118,17 @@ DispatchCustomDOMEvent(Element* aFrameEl
                          /* cancelable = */ true,
                          aDetailValue,
                          res);
   if (res.Failed()) {
     return false;
   }
   customEvent->SetTrusted(true);
   // Dispatch the event.
-  *aStatus = nsEventStatus_eConsumeNoDefault;
+  // We don't initialize aStatus here, as our callers have already done so.
   nsresult rv =
     EventDispatcher::DispatchDOMEvent(aFrameElement, nullptr,
                                       domEvent, presContext, aStatus);
   return NS_SUCCEEDED(rv);
 }
 
 } // namespace
 
@@ -177,17 +177,17 @@ BrowserElementParent::DispatchOpenWindow
 
   // Do not dispatch a mozbrowseropenwindow event of a widget to its embedder
   nsCOMPtr<nsIMozBrowserFrame> browserFrame =
     do_QueryInterface(aOpenerFrameElement);
   if (browserFrame && browserFrame->GetReallyIsWidget()) {
     return BrowserElementParent::OPEN_WINDOW_CANCELLED;
   }
 
-  nsEventStatus status;
+  nsEventStatus status = nsEventStatus_eIgnore;
   bool dispatchSucceeded =
     DispatchCustomDOMEvent(aOpenerFrameElement,
                            NS_LITERAL_STRING("mozbrowseropenwindow"),
                            cx,
                            val, &status);
 
   if (dispatchSucceeded) {
     if (aPopupFrameElement->IsInDoc()) {
--- a/webapprt/content/webapp.js
+++ b/webapprt/content/webapp.js
@@ -15,21 +15,16 @@ XPCOMUtils.defineLazyGetter(this, "gAppB
                             function() document.getElementById("content"));
 
 #ifdef MOZ_CRASHREPORTER
 XPCOMUtils.defineLazyServiceGetter(this, "gCrashReporter",
                                    "@mozilla.org/toolkit/crash-reporter;1",
                                    "nsICrashReporter");
 #endif
 
-function isSameOrigin(url) {
-  let origin = Services.io.newURI(url, null, null).prePath;
-  return (origin == WebappRT.config.app.origin);
-}
-
 let progressListener = {
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIWebProgressListener,
                                          Ci.nsISupportsWeakReference]),
   onLocationChange: function onLocationChange(progress, request, location,
                                               flags) {
 
     // Close tooltip (code adapted from /browser/base/content/browser.js)
     let pageTooltip = document.getElementById("contentAreaTooltip");
@@ -46,73 +41,84 @@ let progressListener = {
           if (tooltipWindow == progress.DOMWindow) {
             pageTooltip.hidePopup();
             break;
           }
         }
       }
     }
 
+    let isSameOrigin = (location.prePath === WebappRT.config.app.origin);
+
     // Set the title of the window to the name of the webapp, adding the origin
     // of the page being loaded if it's from a different origin than the app
     // (per security bug 741955, which specifies that other-origin pages loaded
     // in runtime windows must be identified in chrome).
     let title = WebappRT.localeManifest.name;
-    if (!isSameOrigin(location.spec)) {
+    if (!isSameOrigin) {
       title = location.prePath + " - " + title;
     }
     document.documentElement.setAttribute("title", title);
+
+#ifndef XP_WIN
+#ifndef XP_MACOSX
+    if (isSameOrigin) {
+      // On non-Windows platforms, we open new windows in fullscreen mode
+      // if the opener window is in fullscreen mode, so we hide the menubar;
+      // but on Mac we don't need to hide the menubar.
+      if (document.mozFullScreenElement) {
+        document.getElementById("main-menubar").style.display = "none";
+      }
+    }
+#endif
+#endif
   },
 
   onStateChange: function onStateChange(aProgress, aRequest, aFlags, aStatus) {
     if (aRequest instanceof Ci.nsIChannel &&
         aFlags & Ci.nsIWebProgressListener.STATE_START &&
         aFlags & Ci.nsIWebProgressListener.STATE_IS_DOCUMENT) {
       updateCrashReportURL(aRequest.URI);
     }
   }
 };
 
 function onOpenWindow(event) {
-  let name = event.detail.name;
+  if (event.detail.name === "_blank") {
+    let uri = Services.io.newURI(event.detail.url, null, null);
 
-  if (name == "_blank") {
-    let uri = Services.io.newURI(event.detail.url, null, null);
+    // Prevent the default handler so nsContentTreeOwner.ProvideWindow
+    // doesn't create the window itself.
+    event.preventDefault();
 
     // Direct the URL to the browser.
     Cc["@mozilla.org/uriloader/external-protocol-service;1"].
     getService(Ci.nsIExternalProtocolService).
     getProtocolHandlerInfo(uri.scheme).
     launchWithURI(uri);
-  } else {
-    let win = window.openDialog("chrome://webapprt/content/webapp.xul",
-                                name,
-                                "chrome,dialog=no,resizable," + event.detail.features);
+  }
 
-    win.addEventListener("load", function onLoad() {
-      win.removeEventListener("load", onLoad, false);
+  // Otherwise, don't do anything to make nsContentTreeOwner.ProvideWindow
+  // create the window itself and return it to the window.open caller.
+}
 
-#ifndef XP_WIN
-#ifndef XP_MACOSX
-      if (isSameOrigin(event.detail.url)) {
-        // On non-Windows platforms, we open new windows in fullscreen mode
-        // if the opener window is in fullscreen mode, so we hide the menubar;
-        // but on Mac we don't need to hide the menubar.
-        if (document.mozFullScreenElement) {
-          win.document.getElementById("main-menubar").style.display = "none";
-        }
-      }
-#endif
-#endif
-
-      win.document.getElementById("content").docShell.setIsApp(WebappRT.appID);
-      win.document.getElementById("content").setAttribute("src", event.detail.url);
-    }, false);
+function onDOMContentLoaded() {
+  window.removeEventListener("DOMContentLoaded", onDOMContentLoaded, false);
+  // The initial window's app ID is set by Startup.jsm before the app
+  // is loaded, so this code only handles subsequent windows that are opened
+  // by the app via window.open calls.  We do this on DOMContentLoaded
+  // in order to ensure it gets set before the window's content is loaded.
+  if (gAppBrowser.docShell.appId === Ci.nsIScriptSecurityManager.NO_APP_ID) {
+    // Set the principal to the correct app ID.  Since this is a subsequent
+    // window, we know that WebappRT.configPromise has been resolved, so we
+    // don't have to yield to it before accessing WebappRT.appID.
+    gAppBrowser.docShell.setIsApp(WebappRT.appID);
   }
 }
+window.addEventListener("DOMContentLoaded", onDOMContentLoaded, false);
 
 function onLoad() {
   window.removeEventListener("load", onLoad, false);
 
   gAppBrowser.addProgressListener(progressListener,
                                   Ci.nsIWebProgress.NOTIFY_LOCATION |
                                   Ci.nsIWebProgress.NOTIFY_STATE_DOCUMENT);
 
--- a/webapprt/test/chrome/browser_window-open.js
+++ b/webapprt/test/chrome/browser_window-open.js
@@ -22,17 +22,22 @@ function test() {
       win.addEventListener("load", function onLoadWindow() {
         win.removeEventListener("load", onLoadWindow, false);
 
         if (win.location == "chrome://webapprt/content/webapp.xul") {
           let winAppBrowser = win.document.getElementById("content");
           winAppBrowser.addEventListener("load", function onLoadBrowser() {
             winAppBrowser.removeEventListener("load", onLoadBrowser, true);
 
-            is(winAppBrowser.getAttribute("src"),
+            let contentWindow = Cu.waiveXrays(gAppBrowser.contentDocument.defaultView);
+            is(contentWindow.openedWindow.location.href,
+               "http://test/webapprtChrome/webapprt/test/chrome/sample.html",
+               "window.open returns window with correct URL");
+
+            is(winAppBrowser.documentURI.spec,
                "http://test/webapprtChrome/webapprt/test/chrome/sample.html",
                "New window browser has correct src");
 
             is(winAppBrowser.contentDocument.defaultView.document.nodePrincipal.appId,
                appID,
                "New window principal app ID correct");
 
             win.close();
--- a/webapprt/test/chrome/window-open.html
+++ b/webapprt/test/chrome/window-open.html
@@ -1,15 +1,16 @@
 <!DOCTYPE HTML>
 <html>
   <head>
     <title>Window Open Test App</title>
     <meta charset="utf-8">
     <script>
+      var openedWindow;
       function onLoad() {
-        window.open("sample.html");
+        openedWindow = window.open("sample.html");
       }
     </script>
   </head>
   <body onload="onLoad()">
     <h1>Window Open Test App</h1>
   </body>
 </html>