Bug 1044351 - prevent popup blocker from blocking windows opened by loaded javascript: URIs by allowing popups for such loads from the location bar. r=mak/mrbkap
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Tue, 16 Jun 2015 16:26:36 -0400
changeset 249238 f50a771d7d1b07635934905e6cd15b6b03254c8a
parent 249237 c1b33c43f0c5debb3968d236c32b80a86d79c13a
child 249239 25e99bc12482eb4a72abc512bbbe1aecc61afcfd
push id28923
push userryanvm@gmail.com
push dateWed, 17 Jun 2015 18:57:11 +0000
treeherdermozilla-central@099d6cd6725e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak, mrbkap
bugs1044351
milestone41.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 1044351 - prevent popup blocker from blocking windows opened by loaded javascript: URIs by allowing popups for such loads from the location bar. r=mak/mrbkap
browser/base/content/urlbarBindings.xml
browser/base/content/utilityOverlay.js
browser/components/places/PlacesUIUtils.jsm
browser/components/places/tests/browser/browser.ini
browser/components/places/tests/browser/browser_bookmarklet_windowOpen.js
browser/components/places/tests/browser/pageopeningwindow.html
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -346,17 +346,18 @@ file, You can obtain one at http://mozil
               Cu.reportError(ex);
             }
 
             let loadCurrent = () => {
               openUILinkIn(url, "current", {
                 allowThirdPartyFixup: true,
                 disallowInheritPrincipal: !mayInheritPrincipal,
                 allowPinnedTabHostChange: true,
-                postData: postData
+                postData: postData,
+                allowPopups: url.startsWith("javascript:"),
               });
               // Ensure the start of the URL is visible for UX reasons:
               this.selectionStart = this.selectionEnd = 0;
             };
 
             // Focus the content area before triggering loads, since if the load
             // occurs in a new tab, we want focus to be restored to the content
             // area when the current tab is re-selected.
--- a/browser/base/content/utilityOverlay.js
+++ b/browser/base/content/utilityOverlay.js
@@ -183,16 +183,17 @@ function whereToOpenLink( e, ignoreButto
  * Instead of aAllowThirdPartyFixup, you may also pass an object with any of
  * these properties:
  *   allowThirdPartyFixup (boolean)
  *   postData             (nsIInputStream)
  *   referrerURI          (nsIURI)
  *   relatedToCurrent     (boolean)
  *   skipTabAnimation     (boolean)
  *   allowPinnedTabHostChange (boolean)
+ *   allowPopups          (boolean)
  */
 function openUILinkIn(url, where, aAllowThirdPartyFixup, aPostData, aReferrerURI) {
   var params;
 
   if (arguments.length == 3 && typeof arguments[2] == "object") {
     params = aAllowThirdPartyFixup;
   } else {
     params = {
@@ -225,16 +226,17 @@ function openLinkIn(url, where, params) 
   var aAllowMixedContent    = params.allowMixedContent;
   var aInBackground         = params.inBackground;
   var aDisallowInheritPrincipal = params.disallowInheritPrincipal;
   var aInitiatingDoc        = params.initiatingDoc;
   var aIsPrivate            = params.private;
   var aSkipTabAnimation     = params.skipTabAnimation;
   var aAllowPinnedTabHostChange = !!params.allowPinnedTabHostChange;
   var aNoReferrer           = params.noReferrer;
+  var aAllowPopups          = !!params.allowPopups;
 
   if (where == "save") {
     if (!aInitiatingDoc) {
       Components.utils.reportError("openUILink/openLinkIn was called with " +
         "where == 'save' but without initiatingDoc.  See bug 814264.");
       return;
     }
     // TODO(1073187): propagate referrerPolicy.
@@ -337,18 +339,23 @@ function openLinkIn(url, where, params) 
       flags |= Ci.nsIWebNavigation.LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP;
       flags |= Ci.nsIWebNavigation.LOAD_FLAGS_FIXUP_SCHEME_TYPOS;
     }
 
     // LOAD_FLAGS_DISALLOW_INHERIT_OWNER isn't supported for javascript URIs,
     // i.e. it causes them not to load at all. Callers should strip
     // "javascript:" from pasted strings to protect users from malicious URIs
     // (see stripUnsafeProtocolOnPaste).
-    if (aDisallowInheritPrincipal && !(uriObj && uriObj.schemeIs("javascript")))
+    if (aDisallowInheritPrincipal && !(uriObj && uriObj.schemeIs("javascript"))) {
       flags |= Ci.nsIWebNavigation.LOAD_FLAGS_DISALLOW_INHERIT_OWNER;
+    }
+
+    if (aAllowPopups) {
+      flags |= Ci.nsIWebNavigation.LOAD_FLAGS_ALLOW_POPUPS;
+    }
 
     w.gBrowser.loadURIWithFlags(url, {
       flags: flags,
       referrerURI: aNoReferrer ? null : aReferrerURI,
       referrerPolicy: aReferrerPolicy,
       postData: aPostData,
     });
     break;
--- a/browser/components/places/PlacesUIUtils.jsm
+++ b/browser/components/places/PlacesUIUtils.jsm
@@ -863,16 +863,17 @@ this.PlacesUIUtils = {
           if (browserWin) {
             browserWin.openWebPanel(aNode.title, aNode.uri);
             return;
           }
         }
       }
 
       aWindow.openUILinkIn(aNode.uri, aWhere, {
+        allowPopups: aNode.uri.startsWith("javascript:"),
         inBackground: Services.prefs.getBoolPref("browser.tabs.loadBookmarksInBackground"),
         private: aPrivate,
       });
     }
   },
 
   /**
    * Helper for guessing scheme from an url string.
--- a/browser/components/places/tests/browser/browser.ini
+++ b/browser/components/places/tests/browser/browser.ini
@@ -16,16 +16,19 @@ support-files =
 [browser_410196_paste_into_tags.js]
 [browser_416459_cut.js]
 [browser_423515.js]
 [browser_425884.js]
 [browser_435851_copy_query.js]
 [browser_475045.js]
 [browser_555547.js]
 [browser_bookmarkProperties_addKeywordForThisSearch.js]
+[browser_bookmarklet_windowOpen.js]
+support-files =
+  pageopeningwindow.html
 [browser_bookmarksProperties.js]
 [browser_drag_bookmarks_on_toolbar.js]
 skip-if = e10s # Bug ?????? - test fails - "Number of dragged items should be the same. - Got 0, expected 1"
 [browser_forgetthissite_single.js]
 [browser_history_sidebar_search.js]
 skip-if = e10s && (os == 'linux' || os == 'mac') # Bug 1116457
 [browser_library_batch_delete.js]
 [browser_library_commands.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/places/tests/browser/browser_bookmarklet_windowOpen.js
@@ -0,0 +1,61 @@
+"use strict";
+
+const TEST_URL = 'http://example.com/browser/browser/components/places/tests/browser/pageopeningwindow.html';
+
+function makeBookmarkFor(url, keyword) {
+  return Promise.all([
+    PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+                                   title: "bookmarklet",
+                                   url: url }),
+    PlacesUtils.keywords.insert({url: url,
+                                 keyword: keyword})
+  ]);
+
+}
+
+add_task(function openKeywordBookmarkWithWindowOpen() {
+  // This is the current default, but let's not assume that...
+  yield new Promise((resolve, reject) => {
+    SpecialPowers.pushPrefEnv({ 'set': [[ 'browser.link.open_newwindow', 3 ],
+                                        [ 'dom.disable_open_during_load', true ]] },
+                              resolve);
+  });
+
+  let moztab;
+  let tabOpened = BrowserTestUtils.openNewForegroundTab(gBrowser, "about:mozilla")
+                    .then((tab) => { moztab = tab; });
+  let keywordForBM = "openmeatab";
+
+  let bookmarkInfo;
+  let bookmarkCreated =
+    makeBookmarkFor("javascript:void open('" + TEST_URL + "')", keywordForBM)
+        .then((values) => {
+          bookmarkInfo = values[0];
+        });
+  yield Promise.all([tabOpened, bookmarkCreated]);
+
+  registerCleanupFunction(function() {
+    return Promise.all([
+      PlacesUtils.bookmarks.remove(bookmarkInfo),
+      PlacesUtils.keywords.remove(keywordForBM)
+    ]);
+  });
+  gURLBar.value = keywordForBM;
+  gURLBar.focus();
+
+  let tabCreatedPromise = BrowserTestUtils.waitForEvent(gBrowser.tabContainer, "TabOpen");
+  EventUtils.synthesizeKey("VK_RETURN", {});
+
+  info("Waiting for tab being created");
+  let {target: tab} = yield tabCreatedPromise;
+  info("Got tab");
+  let browser = tab.linkedBrowser;
+  if (!browser.currentURI || browser.currentURI.spec != TEST_URL) {
+    info("Waiting for browser load");
+    yield BrowserTestUtils.browserLoaded(browser);
+  }
+  is(browser.currentURI && browser.currentURI.spec, TEST_URL, "Tab with expected URL loaded.");
+  info("Waiting to remove tab");
+  yield Promise.all([ BrowserTestUtils.removeTab(tab),
+                      BrowserTestUtils.removeTab(moztab) ]);
+});
new file mode 100644
--- /dev/null
+++ b/browser/components/places/tests/browser/pageopeningwindow.html
@@ -0,0 +1,9 @@
+<meta charset="UTF-8">
+Hi, I was opened via a <script>document.write(location.search ?
+  "popup call from the opened window... uh oh, that shouldn't happen!" :
+  "bookmarklet, and I will open a new window myself.")</script><br>
+<script>
+  if (!location.search) {
+    open(location.href + "?donotopen=true", '_blank');
+  }
+</script>