From ec0280ebc4e131ed60b62d967b6276e65db3beef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Aragunde=20P=C3=A9rez?= Date: Tue, 19 Jan 2016 11:32:35 -0800 Subject: [PATCH] Bug 989288: Use toplevel window when handling windows events. r=zer0 --- .../source/lib/sdk/deprecated/window-utils.js | 6 +-- addon-sdk/source/lib/sdk/windows/firefox.js | 14 ++++--- .../source/test/tabs/test-firefox-tabs.js | 38 ++++++++++--------- addon-sdk/source/test/test-page-mod.js | 19 ++++++++++ .../test/windows/test-firefox-windows.js | 31 +++++++++++++++ 5 files changed, 82 insertions(+), 26 deletions(-) diff --git a/addon-sdk/source/lib/sdk/deprecated/window-utils.js b/addon-sdk/source/lib/sdk/deprecated/window-utils.js index 1048322d4fc..4f3217d38a4 100644 --- a/addon-sdk/source/lib/sdk/deprecated/window-utils.js +++ b/addon-sdk/source/lib/sdk/deprecated/window-utils.js @@ -10,7 +10,7 @@ module.metadata = { const { Cc, Ci } = require('chrome'); const events = require('../system/events'); const { getInnerId, getOuterId, windows, isDocumentLoaded, isBrowser, - getMostRecentBrowserWindow, getMostRecentWindow } = require('../window/utils'); + getMostRecentBrowserWindow, getToplevelWindow, getMostRecentWindow } = require('../window/utils'); const { deprecateFunction } = require('../util/deprecate'); const { ignoreWindow } = require('sdk/private-browsing/utils'); const { isPrivateBrowsingSupported } = require('../self'); @@ -127,7 +127,7 @@ WindowTracker.prototype = { if (event.type == 'load' && event.target) { var window = event.target.defaultView; if (window) - this._regWindow(window); + this._regWindow(getToplevelWindow(window)); } } catch(e) { @@ -136,7 +136,7 @@ WindowTracker.prototype = { }, _onToplevelWindowReady: function _onToplevelWindowReady({subject}) { - let window = subject; + let window = getToplevelWindow(subject); // ignore private windows if they are not supported if (ignoreWindow(window)) return; diff --git a/addon-sdk/source/lib/sdk/windows/firefox.js b/addon-sdk/source/lib/sdk/windows/firefox.js index a4dd8b554b1..0c44927157c 100644 --- a/addon-sdk/source/lib/sdk/windows/firefox.js +++ b/addon-sdk/source/lib/sdk/windows/firefox.js @@ -6,7 +6,7 @@ const { Class } = require('../core/heritage'); const { observer } = require('./observer'); const { isBrowser, getMostRecentBrowserWindow, windows, open, getInnerId, - getWindowTitle, isFocused, isWindowPrivate } = require('../window/utils'); + getWindowTitle, getToplevelWindow, isFocused, isWindowPrivate } = require('../window/utils'); const { List, addListItem, removeListItem } = require('../util/list'); const { viewFor } = require('../view/core'); const { modelFor } = require('../model/core'); @@ -189,14 +189,16 @@ for (let domWindow of windows()) { } var windowEventListener = (event, domWindow, ...args) => { - if (ignoreWindow(domWindow)) + let toplevelWindow = getToplevelWindow(domWindow); + + if (ignoreWindow(toplevelWindow)) return; - let window = modelsFor.get(domWindow); + let window = modelsFor.get(toplevelWindow); if (!window) - window = makeNewWindow(domWindow); + window = makeNewWindow(toplevelWindow); - if (isBrowser(domWindow)) { + if (isBrowser(toplevelWindow)) { if (event == "open") addListItem(browserWindows, window); else if (event == "close") @@ -208,7 +210,7 @@ var windowEventListener = (event, domWindow, ...args) => { // The window object shouldn't be reachable after closed if (event == "close") { viewsFor.delete(window); - modelsFor.delete(domWindow); + modelsFor.delete(toplevelWindow); } }; observer.on("*", windowEventListener); diff --git a/addon-sdk/source/test/tabs/test-firefox-tabs.js b/addon-sdk/source/test/tabs/test-firefox-tabs.js index 0c9fd418739..368ed02ba90 100644 --- a/addon-sdk/source/test/tabs/test-firefox-tabs.js +++ b/addon-sdk/source/test/tabs/test-firefox-tabs.js @@ -22,6 +22,7 @@ const DISABLE_POPUP_PREF = 'dom.disable_open_during_load'; const fixtures = require("../fixtures"); const { base64jpeg } = fixtures; const { cleanUI, before, after } = require("sdk/test/utils"); +const { wait } = require('../event/helpers'); // Bug 682681 - tab.title should never be empty exports.testBug682681_aboutURI = function(assert, done) { @@ -1214,32 +1215,35 @@ exports['test active tab properties defined on popup closed'] = function (assert }); }; -// related to bug 922956 +// related to bugs 922956 and 989288 // https://bugzilla.mozilla.org/show_bug.cgi?id=922956 -exports["test ready event after window.open"] = function (assert, done) { +// https://bugzilla.mozilla.org/show_bug.cgi?id=989288 +exports["test tabs ready and close after window.open"] = function*(assert, done) { + // ensure popups open in a new window and disable popup blocker setPref(OPEN_IN_NEW_WINDOW_PREF, 2); setPref(DISABLE_POPUP_PREF, false); - let firstRun = true; - tabs.on('ready', function onReady(tab) { - if (firstRun) { - assert.pass("tab ready callback after 1st window.open"); - firstRun = false; - tab.close(); - } - else { - assert.pass("tab ready callback after 2nd window.open"); - tabs.removeListener('ready', onReady); - tab.close(done); - } - }); - + // open windows to trigger observers tabs.activeTab.attach({ contentScript: "window.open('about:blank');" + "window.open('about:blank', '', " + "'width=800,height=600,resizable=no,status=no,location=no');" }); -} + + let tab1 = yield wait(tabs, "ready"); + assert.pass("first tab ready has occured"); + + let tab2 = yield wait(tabs, "ready"); + assert.pass("second tab ready has occured"); + + tab1.close(); + yield wait(tabs, "close"); + assert.pass("first tab close has occured"); + + tab2.close(); + yield wait(tabs, "close"); + assert.pass("second tab close has occured"); +}; // related to bug #939496 exports["test tab open event for new window"] = function(assert, done) { diff --git a/addon-sdk/source/test/test-page-mod.js b/addon-sdk/source/test/test-page-mod.js index ce8c1679bfb..d03463d2db2 100644 --- a/addon-sdk/source/test/test-page-mod.js +++ b/addon-sdk/source/test/test-page-mod.js @@ -586,6 +586,25 @@ exports.testRelatedTab = function(assert, done) { }); }; +// related to bug #989288 +// https://bugzilla.mozilla.org/show_bug.cgi?id=989288 +exports.testRelatedTabNewWindow = function(assert, done) { + let url = "about:logo" + let pageMod = new PageMod({ + include: url, + onAttach: function(worker) { + assert.equal(worker.tab.url, url, "Worker.tab.url is valid"); + worker.tab.close(done); + } + }); + + tabs.activeTab.attach({ + contentScript: "window.open('about:logo', '', " + + "'width=800,height=600,resizable=no,status=no,location=no');" + }); + +}; + exports.testRelatedTabNoRequireTab = function(assert, done) { let loader = Loader(module); let tab; diff --git a/addon-sdk/source/test/windows/test-firefox-windows.js b/addon-sdk/source/test/windows/test-firefox-windows.js index a2ffccabe6d..578def054aa 100644 --- a/addon-sdk/source/test/windows/test-firefox-windows.js +++ b/addon-sdk/source/test/windows/test-firefox-windows.js @@ -20,6 +20,10 @@ const { after } = require("sdk/test/utils"); const { merge } = require("sdk/util/object"); const self = require("sdk/self"); const { openTab } = require("../tabs/utils"); +const { set: setPref, reset: resetPref } = require("sdk/preferences/service"); +const OPEN_IN_NEW_WINDOW_PREF = 'browser.link.open_newwindow'; +const DISABLE_POPUP_PREF = 'dom.disable_open_during_load'; +const { wait } = require('../event/helpers'); // TEST: open & close window exports.testOpenAndCloseWindow = function(assert, done) { @@ -583,8 +587,35 @@ exports.testWindowTabEventBindings = function(assert, done) { windowTabs.open(openArgs); } +// related to bug #989288 +// https://bugzilla.mozilla.org/show_bug.cgi?id=989288 +exports["test window events after window.open"] = function*(assert, done) { + // ensure popups open in a new windows and disable popup blocker + setPref(OPEN_IN_NEW_WINDOW_PREF, 2); + setPref(DISABLE_POPUP_PREF, false); + + // open window to trigger observers + tabs.activeTab.attach({ + contentScript: "window.open('about:blank', '', " + + "'width=800,height=600,resizable=no,status=no,location=no');" + }); + + let window = yield wait(browserWindows, "open"); + assert.pass("tab open has occured"); + window.close(); + + yield wait(browserWindows,"close"); + assert.pass("tab close has occured"); +}; + after(exports, function*(name, assert) { + resetPopupPrefs(); yield cleanUI(); }); +const resetPopupPrefs = () => { + resetPref(OPEN_IN_NEW_WINDOW_PREF); + resetPref(DISABLE_POPUP_PREF); +}; + require('sdk/test').run(exports);