From ee8a87ec2a071bf8d98d11cbe15de49367d47ef7 Mon Sep 17 00:00:00 2001 From: Kevin Grandon Date: Thu, 28 Aug 2014 22:32:31 -0700 Subject: [PATCH] Bug 1060191 - Web compat issues with window.open, add opener test case --- apps/system/js/app_window_factory.js | 14 ++++----- apps/system/js/browser.js | 2 +- apps/system/js/browser_config_helper.js | 31 +++++++++++-------- apps/system/js/child_window_factory.js | 18 ++++++++++- apps/system/js/homescreen_window.js | 5 ++- apps/system/js/search_window.js | 5 ++- apps/system/js/secure_window_factory.js | 5 ++- .../browser_launch_window_open_test.js | 26 ++++++++++++++-- .../test/marionette/fixtures/callopener.html | 14 +++++++++ .../test/marionette/fixtures/windowopen.html | 1 + .../test/unit/app_window_factory_test.js | 12 +++++++ .../test/unit/child_window_factory_test.js | 28 ++++++++--------- 12 files changed, 118 insertions(+), 43 deletions(-) create mode 100644 apps/system/test/marionette/fixtures/callopener.html diff --git a/apps/system/js/app_window_factory.js b/apps/system/js/app_window_factory.js index 7296a16bb59e..ecb7623f5b6f 100644 --- a/apps/system/js/app_window_factory.js +++ b/apps/system/js/app_window_factory.js @@ -49,6 +49,7 @@ window.addEventListener('webapps-launch', this.preHandleEvent); window.addEventListener('webapps-close', this.preHandleEvent); window.addEventListener('open-app', this.preHandleEvent); + window.addEventListener('openwindow', this.preHandleEvent); window.addEventListener('appopenwindow', this.preHandleEvent); window.addEventListener('applicationready', (function appReady(e) { window.removeEventListener('applicationready', appReady); @@ -69,6 +70,7 @@ window.removeEventListener('webapps-launch', this.preHandleEvent); window.removeEventListener('webapps-close', this.preHandleEvent); window.removeEventListener('open-app', this.preHandleEvent); + window.removeEventListener('openwindow', this.preHandleEvent); window.removeEventListener('appopenwindow', this.preHandleEvent); }, @@ -98,20 +100,16 @@ handleEvent: function awf_handleEvent(evt) { var detail = evt.detail; - var manifestURL = detail.manifestURL; - if (!manifestURL) { + if (!detail.url) { return; } - var config = new BrowserConfigHelper(detail.url, detail.manifestURL); - - if (!config.manifest) { - return; - } + var config = new BrowserConfigHelper(detail); config.evtType = evt.type; switch (evt.type) { + case 'openwindow': case 'appopenwindow': case 'webapps-launch': config.timestamp = detail.timestamp; @@ -180,7 +178,7 @@ // The rocketbar currently handles the management of normal search app // launches. Requests for the 'newtab' page will continue to filter // through and publish the launchapp event. - if (config.manifest.role === 'search' && + if (config.manifest && config.manifest.role === 'search' && config.url.indexOf('newtab.html') === -1) { return; } diff --git a/apps/system/js/browser.js b/apps/system/js/browser.js index 77cd59f9c256..020cc75b4b6f 100644 --- a/apps/system/js/browser.js +++ b/apps/system/js/browser.js @@ -5,7 +5,7 @@ 'use strict'; function handleOpenUrl(url) { - var config = new BrowserConfigHelper(url); + var config = new BrowserConfigHelper({url: url}); config.useAsyncPanZoom = true; config.oop = true; var newApp = new AppWindow(config); diff --git a/apps/system/js/browser_config_helper.js b/apps/system/js/browser_config_helper.js index ea2e3f7f37e1..0ece04bcf88a 100644 --- a/apps/system/js/browser_config_helper.js +++ b/apps/system/js/browser_config_helper.js @@ -31,14 +31,18 @@ * * * oop: indicate it's running out of process or in process. * - * @param {String} appURL The URL of the app or the page to be opened. - * @param {String} [manifestURL] The manifest URL of the app. - * - * @class BrowserConfigHelper + * @param {Object} [config] Config for creating appWindow. + * @param {String} [config.appURL] The URL of the app or the page to be + * opened. + * @param {String} [config.manifestURL] The manifest URL of the app. + * @param {String} [config.name] - optional The name of the app. + * @param {DOMFRAMEElement} [config.iframe] - optionalThe exisiting frame to + * inject. */ - window.BrowserConfigHelper = function(appURL, manifestURL) { - var app = applications.getByManifestURL(manifestURL); - this.url = appURL; + window.BrowserConfigHelper = function(config) { + var app = config.manifestURL && + applications.getByManifestURL(config.manifestURL); + this.url = config.url; if (app) { var manifest = app.manifest; @@ -65,8 +69,8 @@ } //Remove the origin and / to find if if the url is the entry point - if (path.indexOf('/' + ep) == 0 && - (currentEp.launch_path == path)) { + if (path.indexOf('/' + ep) === 0 && + (currentEp.launch_path === path)) { origin = origin + currentEp.launch_path; name = new ManifestHelper(currentEp).name; for (var key in currentEp) { @@ -92,18 +96,19 @@ ]; if (!isOutOfProcessDisabled && - outOfProcessBlackList.indexOf(manifestURL) === -1) { + outOfProcessBlackList.indexOf(config.manifestURL) === -1) { // FIXME: content shouldn't control this directly this.oop = true; } this.name = name; - this.manifestURL = manifestURL; + this.manifestURL = config.manifestURL; this.origin = origin; this.manifest = manifest; } else { - this.name = ''; - this.origin = appURL; + this.iframe = config.iframe; + this.name = config.name || ''; + this.origin = config.url; this.manifestURL = ''; this.manifest = null; } diff --git a/apps/system/js/child_window_factory.js b/apps/system/js/child_window_factory.js index 51a9b403fc3f..43a8eccd05ca 100644 --- a/apps/system/js/child_window_factory.js +++ b/apps/system/js/child_window_factory.js @@ -57,7 +57,7 @@ // should never be part of the app if (evt.detail.name == '_blank' && evt.detail.features !== 'attention') { - this.launchActivity(evt); + this.createNewWindow(evt); evt.stopPropagation(); return; } @@ -132,6 +132,22 @@ return (a[0] === b[0] && a[2] === b[2]); }; + ChildWindowFactory.prototype.createNewWindow = function(evt) { + if (!this.app.isActive() || this.app.isTransitioning()) { + return false; + } + var configObject = { + url: evt.detail.url, + name: evt.detail.name, + iframe: evt.detail.frameElement + }; + + window.dispatchEvent(new CustomEvent('openwindow', { + detail: configObject + })); + return true; + }; + ChildWindowFactory.prototype.createChildWindow = function(evt) { if (!this.app.isActive() || this.app.isTransitioning()) { return false; diff --git a/apps/system/js/homescreen_window.js b/apps/system/js/homescreen_window.js index 656a186a3fac..12881608e47b 100644 --- a/apps/system/js/homescreen_window.js +++ b/apps/system/js/homescreen_window.js @@ -86,7 +86,10 @@ this.url = app.origin + '/index.html#root'; this.browser_config = - new BrowserConfigHelper(this.origin, this.manifestURL); + new BrowserConfigHelper({ + url: this.origin, + manifestURL: this.manifestURL + }); // Necessary for b2gperf now. this.name = this.browser_config.name; diff --git a/apps/system/js/search_window.js b/apps/system/js/search_window.js index 036f3c1ada56..f7cbad528344 100644 --- a/apps/system/js/search_window.js +++ b/apps/system/js/search_window.js @@ -84,7 +84,10 @@ this.url = app.origin + '/index.html'; this.browser_config = - new BrowserConfigHelper(this.origin, this.manifestURL); + new BrowserConfigHelper({ + url: this.origin, + manifestURL: this.manifestURL + }); this.manifest = this.browser_config.manifest; this.browser_config.url = this.url; diff --git a/apps/system/js/secure_window_factory.js b/apps/system/js/secure_window_factory.js index 6ab0c407bdb7..26badeda98f5 100644 --- a/apps/system/js/secure_window_factory.js +++ b/apps/system/js/secure_window_factory.js @@ -139,7 +139,10 @@ */ SecureWindowFactory.prototype.create = function(url, manifestURL) { - var config = new self.BrowserConfigHelper(url, manifestURL); + var config = new self.BrowserConfigHelper({ + url: url, + manifestURL: manifestURL + }); for (var instanceID in this.states.apps) { var secureWindow = this.states.apps[instanceID]; if (config.manifestURL === secureWindow.manifestURL) { diff --git a/apps/system/test/marionette/browser_launch_window_open_test.js b/apps/system/test/marionette/browser_launch_window_open_test.js index 778436fd538c..b0858678bacd 100644 --- a/apps/system/test/marionette/browser_launch_window_open_test.js +++ b/apps/system/test/marionette/browser_launch_window_open_test.js @@ -21,7 +21,7 @@ marionette('Browser - Launch the same origin after navigating away', 'lockscreen.enabled': false } }); - + client.scope({ searchTimeout: 20000 }); var home, rocketbar, search, server, system; suiteSetup(function(done) { @@ -58,7 +58,7 @@ marionette('Browser - Launch the same origin after navigating away', client.switchToFrame(frame); client.switchToFrame(); - var browsers = client.findElements( 'iframe[src*="http://localhost"]'); + var browsers = client.findElements( 'iframe[data-url*="http://localhost"]'); assert.equal(browsers.length, 1); client.switchToFrame(frame); @@ -68,8 +68,28 @@ marionette('Browser - Launch the same origin after navigating away', client.switchToFrame(); client.waitFor(function() { - browsers = client.findElements('iframe[src*="http://localhost"]'); + browsers = client.findElements('iframe[data-url*="http://localhost"]'); return browsers.length === 2; }); }); + + test('checks for web compat issues', function() { + var url = server.url('windowopen.html'); + // rest condition, the alert might be show up quciker than new page + // Open the first URL in a sheet. + rocketbar.homescreenFocus(); + rocketbar.enterText(url + '\uE006'); + + // Switch to the app, and navigate to a different url. + system.gotoBrowser(url); + client.helper.waitForElement('#trigger2').tap(); + client.switchToFrame(); + + client.waitFor(function() { + return client.findElement( + '.appWindow .modal-dialog-alert-message') + .text() + .indexOf('caller received') !== -1; + }); + }); }); diff --git a/apps/system/test/marionette/fixtures/callopener.html b/apps/system/test/marionette/fixtures/callopener.html new file mode 100644 index 000000000000..be51f3211eb1 --- /dev/null +++ b/apps/system/test/marionette/fixtures/callopener.html @@ -0,0 +1,14 @@ + + + Hyperlink Test Page + + + + + + Calling opener alert. + + diff --git a/apps/system/test/marionette/fixtures/windowopen.html b/apps/system/test/marionette/fixtures/windowopen.html index d59ecca702fd..872c84701368 100644 --- a/apps/system/test/marionette/fixtures/windowopen.html +++ b/apps/system/test/marionette/fixtures/windowopen.html @@ -8,6 +8,7 @@ diff --git a/apps/system/test/unit/app_window_factory_test.js b/apps/system/test/unit/app_window_factory_test.js index bed24c17acd8..8fae9ce39fef 100644 --- a/apps/system/test/unit/app_window_factory_test.js +++ b/apps/system/test/unit/app_window_factory_test.js @@ -283,6 +283,18 @@ suite('system/AppWindowFactory', function() { fakeLaunchConfig2.url); }); + test('open a new window', function() { + var stubDispatchEvent = this.sinon.stub(window, 'dispatchEvent'); + appWindowFactory.handleEvent({ + type: 'openwindow', + detail: fakeLaunchConfig2 + }); + assert.isTrue(stubDispatchEvent.called); + assert.equal(stubDispatchEvent.getCall(0).args[0].type, 'launchapp'); + assert.equal(stubDispatchEvent.getCall(0).args[0].detail.url, + fakeLaunchConfig2.url); + }); + test('opening from a system message', function() { var stubDispatchEvent = this.sinon.stub(window, 'dispatchEvent'); appWindowFactory.handleEvent({ diff --git a/apps/system/test/unit/child_window_factory_test.js b/apps/system/test/unit/child_window_factory_test.js index cb56624605cc..655a2419ea04 100644 --- a/apps/system/test/unit/child_window_factory_test.js +++ b/apps/system/test/unit/child_window_factory_test.js @@ -254,21 +254,21 @@ suite('system/ChildWindowFactory', function() { test('Target _blank support', function() { var app1 = new MockAppWindow(fakeAppConfig1); - var activitySpy = this.sinon.spy(window, 'MozActivity'); var cwf = new ChildWindowFactory(app1); - var expectedActivity = { - name: 'view', - data: { - type: 'url', - url: 'http://blank.com/index.html' - } - }; - cwf.handleEvent(new CustomEvent('mozbrowseropenwindow', - { - detail: fakeWindowOpenBlank - })); - assert.isTrue(activitySpy.calledWithNew()); - sinon.assert.calledWith(activitySpy, expectedActivity); + this.sinon.stub(app1, 'isActive').returns(true); + this.sinon.stub(app1, 'isTransitioning').returns(false); + var stubDispatchEvent = this.sinon.stub(window, 'dispatchEvent'); + var testEvt = (new CustomEvent('mozbrowseropenwindow', { + detail: fakeWindowOpenBlank + })); + cwf.handleEvent(testEvt); + + assert.equal(stubDispatchEvent.getCall(0).args[0].type, 'openwindow'); + assert.deepEqual(stubDispatchEvent.getCall(0).args[0].detail, { + url: fakeWindowOpenBlank.url, + name: fakeWindowOpenBlank.name, + iframe: fakeWindowOpenBlank.frameElement + }); }); test('Create ActivityWindow', function() {