From ad263a1b539660b00e4cd4d4c5ac7be38acf9351 Mon Sep 17 00:00:00 2001
From: Jason Chase <jasonpchase@gmail.com>
Date: Thu, 12 Feb 2015 16:34:32 -0500
Subject: [PATCH] CB-8444 Don't clobber `window.open` - Add new symbol/clobber
 to access open function (`cordova.InAppBrowser.open`) - Change existing tests
 to use new symbol (i.e. don't rely on plugin clobber of `window.open`) - Add
 tests to use `window.open` via manual replace with new symbol - Update docs
 to deprecate plugin clobber of `window.open`

---
 doc/index.md   | 45 +++++++++++++++++++++++++------------
 plugin.xml     |  9 ++++++++
 tests/tests.js | 61 ++++++++++++++++++++++++++++++++++++++++++--------
 3 files changed, 92 insertions(+), 23 deletions(-)

diff --git a/doc/index.md b/doc/index.md
index a6fb07a..598fdb3 100644
--- a/doc/index.md
+++ b/doc/index.md
@@ -19,9 +19,15 @@
 
 # org.apache.cordova.inappbrowser
 
-This plugin provides a web browser view that displays when calling `window.open()`.
+This plugin provides a web browser view that displays when calling `cordova.InAppBrowser.open()`.
 
-    var ref = window.open('http://apache.org', '_blank', 'location=yes');
+    var ref = cordova.InAppBrowser.open('http://apache.org', '_blank', 'location=yes');
+
+The `cordova.InAppBrowser.open()` function is defined to be a drop-in replacement
+for the `window.open()` function.  Existing `window.open()` calls can use the
+InAppBrowser window, by replacing window.open:
+
+    window.open = cordova.InAppBrowser.open;
 
 The InAppBrowser window behaves like a standard web browser,
 and can't access Cordova APIs. For this reason, the InAppBrowser is recommended
@@ -32,7 +38,10 @@ whitelist, nor is opening links in the system browser.
 The InAppBrowser provides by default its own GUI controls for the user (back,
 forward, done).
 
-This plugin hooks `window.open`.
+For backwards compatibility, this plugin also hooks `window.open`.
+However, the plugin-installed hook of `window.open` can have unintended side
+effects (especially if this plugin is included only as a dependency of another
+plugin).  The hook of `window.open` will be removed in a future major release.
 
 Although `window.open` is in the global scope, InAppBrowser is not available until after the `deviceready` event.
 
@@ -45,12 +54,20 @@ Although `window.open` is in the global scope, InAppBrowser is not available unt
 
     cordova plugin add org.apache.cordova.inappbrowser
 
-## window.open
+If you want all page loads in your app to go through the InAppBrowser, you can
+simply hook `window.open` during initialization.  For example:
+
+    document.addEventListener("deviceready", onDeviceReady, false);
+    function onDeviceReady() {
+        window.open = cordova.InAppBrowser.open;
+    }
+
+## cordova.InAppBrowser.open
 
 Opens a URL in a new `InAppBrowser` instance, the current browser
 instance, or the system browser.
 
-    var ref = window.open(url, target, options);
+    var ref = cordova.InAppBrowser.open(url, target, options);
 
 - __ref__: Reference to the `InAppBrowser` window. _(InAppBrowser)_
 
@@ -107,8 +124,8 @@ instance, or the system browser.
 
 ### Example
 
-    var ref = window.open('http://apache.org', '_blank', 'location=yes');
-    var ref2 = window.open(encodeURI('http://ja.m.wikipedia.org/wiki/ハングル'), '_blank', 'location=yes');
+    var ref = cordova.InAppBrowser.open('http://apache.org', '_blank', 'location=yes');
+    var ref2 = cordova.InAppBrowser.open(encodeURI('http://ja.m.wikipedia.org/wiki/ハングル'), '_blank', 'location=yes');
 
 ### Firefox OS Quirks
 
@@ -144,7 +161,7 @@ opened with `target='_blank'`. The rules might look like these
 
 ## InAppBrowser
 
-The object returned from a call to `window.open`.
+The object returned from a call to `cordova.InAppBrowser.open`.
 
 ### Methods
 
@@ -193,7 +210,7 @@ The object returned from a call to `window.open`.
 
 ### Quick Example
 
-    var ref = window.open('http://apache.org', '_blank', 'location=yes');
+    var ref = cordova.InAppBrowser.open('http://apache.org', '_blank', 'location=yes');
     ref.addEventListener('loadstart', function(event) { alert(event.url); });
 
 ## removeEventListener
@@ -224,7 +241,7 @@ The function is passed an `InAppBrowserEvent` object.
 
 ### Quick Example
 
-    var ref = window.open('http://apache.org', '_blank', 'location=yes');
+    var ref = cordova.InAppBrowser.open('http://apache.org', '_blank', 'location=yes');
     var myCallback = function(event) { alert(event.url); }
     ref.addEventListener('loadstart', myCallback);
     ref.removeEventListener('loadstart', myCallback);
@@ -248,7 +265,7 @@ The function is passed an `InAppBrowserEvent` object.
 
 ### Quick Example
 
-    var ref = window.open('http://apache.org', '_blank', 'location=yes');
+    var ref = cordova.InAppBrowser.open('http://apache.org', '_blank', 'location=yes');
     ref.close();
 
 ## show
@@ -268,7 +285,7 @@ The function is passed an `InAppBrowserEvent` object.
 
 ### Quick Example
 
-    var ref = window.open('http://apache.org', '_blank', 'hidden=yes');
+    var ref = cordova.InAppBrowser.open('http://apache.org', '_blank', 'hidden=yes');
     // some time later...
     ref.show();
 
@@ -300,7 +317,7 @@ The function is passed an `InAppBrowserEvent` object.
 
 ### Quick Example
 
-    var ref = window.open('http://apache.org', '_blank', 'location=yes');
+    var ref = cordova.InAppBrowser.open('http://apache.org', '_blank', 'location=yes');
     ref.addEventListener('loadstop', function() {
         ref.executeScript({file: "myscript.js"});
     });
@@ -327,7 +344,7 @@ The function is passed an `InAppBrowserEvent` object.
 
 ### Quick Example
 
-    var ref = window.open('http://apache.org', '_blank', 'location=yes');
+    var ref = cordova.InAppBrowser.open('http://apache.org', '_blank', 'location=yes');
     ref.addEventListener('loadstop', function() {
         ref.insertCSS({file: "mystyles.css"});
     });
diff --git a/plugin.xml b/plugin.xml
index 0410725..3817aca 100644
--- a/plugin.xml
+++ b/plugin.xml
@@ -36,6 +36,7 @@
     <!-- android -->
     <platform name="android">
         <js-module src="www/inappbrowser.js" name="inappbrowser">
+            <clobbers target="cordova.InAppBrowser.open" />
             <clobbers target="window.open" />
         </js-module>
         <config-file target="res/xml/config.xml" parent="/*">
@@ -69,6 +70,7 @@
     <!-- amazon-fireos -->
     <platform name="amazon-fireos">
         <js-module src="www/inappbrowser.js" name="inappbrowser">
+            <clobbers target="cordova.InAppBrowser.open" />
             <clobbers target="window.open" />
         </js-module>
         <config-file target="res/xml/config.xml" parent="/*">
@@ -101,6 +103,7 @@
     <!-- ubuntu -->
     <platform name="ubuntu">
         <js-module src="www/inappbrowser.js" name="inappbrowser">
+            <clobbers target="cordova.InAppBrowser.open" />
             <clobbers target="window.open" />
         </js-module>
         <header-file src="src/ubuntu/inappbrowser.h" />
@@ -113,6 +116,7 @@
     <!-- ios -->
     <platform name="ios">
         <js-module src="www/inappbrowser.js" name="inappbrowser">
+            <clobbers target="cordova.InAppBrowser.open" />
             <clobbers target="window.open" />
         </js-module>  
         <config-file target="config.xml" parent="/*">
@@ -134,6 +138,7 @@
         </config-file>
 
         <js-module src="www/inappbrowser.js" name="inappbrowser">
+            <clobbers target="cordova.InAppBrowser.open" />
             <clobbers target="window.open" />
         </js-module>
         <config-file target="config.xml" parent="/*">
@@ -156,6 +161,7 @@
         </config-file>
                 
         <js-module src="www/inappbrowser.js" name="inappbrowser">
+            <clobbers target="cordova.InAppBrowser.open" />
             <clobbers target="window.open" />
         </js-module>
         <config-file target="config.xml" parent="/*">
@@ -174,6 +180,7 @@
     <!-- windows8 -->
     <platform name="windows8">
         <js-module src="www/inappbrowser.js" name="inappbrowser">
+            <clobbers target="cordova.InAppBrowser.open" />
             <clobbers target="window.open" />
         </js-module>
         <js-module src="www/windows8/InAppBrowserProxy.js" name="InAppBrowserProxy">
@@ -184,6 +191,7 @@
     <!-- windows universal apps (Windows 8.1, Windows Phone 8.1, Windows 8.0) -->
     <platform name="windows">
         <js-module src="www/inappbrowser.js" name="inappbrowser">
+            <clobbers target="cordova.InAppBrowser.open" />
             <clobbers target="window.open" />
         </js-module>
         <js-module src="src/windows/InAppBrowserProxy.js" name="InAppBrowserProxy">
@@ -197,6 +205,7 @@
             <permission name="browser" description="Enables the app to implement a browser in an iframe." privileged="true"/>
         </config-file>
         <js-module src="www/inappbrowser.js" name="inappbrowser">
+            <clobbers target="cordova.InAppBrowser.open" />
             <clobbers target="window.open" />
         </js-module>
         <js-module src="src/firefoxos/InAppBrowserProxy.js" name="InAppBrowserProxy">
diff --git a/tests/tests.js b/tests/tests.js
index 1587b99..a827986 100644
--- a/tests/tests.js
+++ b/tests/tests.js
@@ -26,8 +26,9 @@ window.alert = window.alert || navigator.notification.alert;
 
 exports.defineManualTests = function (contentEl, createActionButton) {
 
-    function doOpen(url, target, params, numExpectedRedirects) {
+    function doOpen(url, target, params, numExpectedRedirects, useWindowOpen) {
         numExpectedRedirects = numExpectedRedirects || 0;
+        useWindowOpen = useWindowOpen || false;
         console.log("Opening " + url);
 
         var counts;
@@ -44,17 +45,25 @@ exports.defineManualTests = function (contentEl, createActionButton) {
         }
         reset();
 
-        var iab = window.open(url, target, params, {
+        var iab;
+        var callbacks = {
             loaderror: logEvent,
             loadstart: logEvent,
             loadstop: logEvent,
             exit: logEvent
-        });
+        };
+        if (useWindowOpen) {
+            console.log('Use window.open() for url');
+            iab = window.open(url, target, params, callbacks);
+        }
+        else {
+            iab = cordova.InAppBrowser.open(url, target, params, callbacks);
+        }
         if (!iab) {
-            alert('window.open returned ' + iab);
+            alert('open returned ' + iab);
             return;
         }
-        
+
         function logEvent(e) {
             console.log('IAB event=' + JSON.stringify(e));
             counts[e.type]++;
@@ -86,7 +95,7 @@ exports.defineManualTests = function (contentEl, createActionButton) {
             if (e.type == 'exit') {
                 var numStopEvents = counts['loadstop'] + counts['loaderror'];
                 if (numStopEvents === 0 && !wasReset) {
-                    alert('Unexpected: browser closed without a loadstop or loaderror.')
+                    alert('Unexpected: browser closed without a loadstop or loaderror.');
                 } else if (numStopEvents > 1) {
                     alert('Unexpected: got multiple loadstop/loaderror events.');
                 }
@@ -96,6 +105,25 @@ exports.defineManualTests = function (contentEl, createActionButton) {
         return iab;
     }
 
+    function doHookOpen(url, target, params, numExpectedRedirects) {
+        var originalFunc = window.open;
+        var wasClobbered = window.hasOwnProperty('open');
+        window.open = cordova.InAppBrowser.open;
+
+        try {
+            doOpen(url, target, params, numExpectedRedirects, true);
+        }
+        finally {
+            if (wasClobbered) {
+                window.open = originalFunc;
+            }
+            else {
+              console.log('just delete, to restore open from prototype');
+                delete window.open;
+            }
+        }
+    }
+
     function openWithStyle(url, cssUrl, useCallback) {
         var iab = doOpen(url, '_blank', 'location=yes');
         var callback = function (results) {
@@ -153,9 +181,9 @@ exports.defineManualTests = function (contentEl, createActionButton) {
     var loadlistener = function (event) { alert('background window loaded '); };
     function openHidden(url, startHidden) {
         var shopt = (startHidden) ? 'hidden=yes' : '';
-        hiddenwnd = window.open(url, 'random_string', shopt);
+        hiddenwnd = cordova.InAppBrowser.open(url, 'random_string', shopt);
         if (!hiddenwnd) {
-            alert('window.open returned ' + hiddenwnd);
+            alert('cordova.InAppBrowser.open returned ' + hiddenwnd);
             return;
         }
         if (startHidden) hiddenwnd.addEventListener('loadstop', loadlistener);
@@ -184,6 +212,8 @@ exports.defineManualTests = function (contentEl, createActionButton) {
     var local_tests = '<h1>Local URL</h1>' +
         '<div id="openLocal"></div>' +
         'Expected result: opens successfully in CordovaWebView.' +
+        '<p/> <div id="openLocalHook"></div>' +
+        'Expected result: opens successfully in CordovaWebView (using hook of window.open()).' +
         '<p/> <div id="openLocalSelf"></div>' +
         'Expected result: opens successfully in CordovaWebView.' +
         '<p/> <div id="openLocalSystem"></div>' +
@@ -202,6 +232,8 @@ exports.defineManualTests = function (contentEl, createActionButton) {
     var white_listed_tests = '<h1>White Listed URL</h1>' +
         '<div id="openWhiteListed"></div>' +
         'Expected result: open successfully in CordovaWebView to cordova.apache.org' +
+        '<p/> <div id="openWhiteListedHook"></div>' +
+        'Expected result: open successfully in CordovaWebView to cordova.apache.org (using hook of window.open())' +
         '<p/> <div id="openWhiteListedSelf"></div>' +
         'Expected result: open successfully in CordovaWebView to cordova.apache.org' +
         '<p/> <div id="openWhiteListedSystem"></div>' +
@@ -215,7 +247,9 @@ exports.defineManualTests = function (contentEl, createActionButton) {
 
     var non_white_listed_tests = '<h1>Non White Listed URL</h1>' +
         '<div id="openNonWhiteListed"></div>' +
-        'Expected result: open successfully in InAppBrowser to apple.com (_self enforces whitelist).' +
+        'Expected result: open successfully in InAppBrowser to apple.com.' +
+        '<p/> <div id="openNonWhiteListedHook"></div>' +
+        'Expected result: open successfully in InAppBrowser to apple.com (using hook of window.open()).' +
         '<p/> <div id="openNonWhiteListedSelf"></div>' +
         'Expected result: open successfully in InAppBrowser to apple.com (_self enforces whitelist).' +
         '<p/> <div id="openNonWhiteListedSystem"></div>' +
@@ -320,6 +354,9 @@ exports.defineManualTests = function (contentEl, createActionButton) {
     createActionButton('target=Default', function () {
         doOpen(localhtml);
     }, 'openLocal');
+    createActionButton('target=Default (window.open)', function () {
+        doHookOpen(localhtml);
+    }, 'openLocalHook');
     createActionButton('target=_self', function () {
         doOpen(localhtml, '_self');
     }, 'openLocalSelf');
@@ -346,6 +383,9 @@ exports.defineManualTests = function (contentEl, createActionButton) {
     createActionButton('* target=Default', function () {
         doOpen('http://cordova.apache.org');
     }, 'openWhiteListed');
+    createActionButton('* target=Default (window.open)', function () {
+        doHookOpen('http://cordova.apache.org');
+    }, 'openWhiteListedHook');
     createActionButton('* target=_self', function () {
         doOpen('http://cordova.apache.org', '_self');
     }, 'openWhiteListedSelf');
@@ -366,6 +406,9 @@ exports.defineManualTests = function (contentEl, createActionButton) {
     createActionButton('target=Default', function () {
         doOpen('http://www.apple.com');
     }, 'openNonWhiteListed');
+    createActionButton('target=Default (window.open)', function () {
+        doHookOpen('http://www.apple.com');
+    }, 'openNonWhiteListedHook');
     createActionButton('target=_self', function () {
         doOpen('http://www.apple.com', '_self');
     }, 'openNonWhiteListedSelf');