Merge pull request #157 from robertknight/wombat-ms-edge-compat

ikreymer · ikreymer · commit b3c9a47ec7da · 2015-11-26T10:11:21.000-08:00
Microsoft Edge compatibility fixes, including Safari fix from #156 and Node.prototype fix from #154
diff --git a/.travis.yml b/.travis.yml
@@ -7,6 +7,9 @@ python:
 os:
   - linux
 
+addons:
+  sauce_connect: true
+
 cache:
   directories:
     - $HOME/.cache/pip
@@ -22,6 +25,10 @@ install:
   - pip install coverage pytest-cov coveralls --use-mirrors
   - npm install
 
+before_script:
+  - export DISPLAY=:99.0
+  - sh -e /etc/init.d/xvfb start
+
 script:
   - python setup.py test
   - cd karma-tests && make test
diff --git a/karma-tests/dummy.html b/karma-tests/dummy.html
@@ -0,0 +1,9 @@
+<html>
+<head><meta charset="UTF-8"></head>
+<body>
+	<!-- This is a dummy page used in
+	     tests of Wombat's live-rewriting
+		 functionality.
+	!-->
+</body>
+</html>
diff --git a/karma-tests/karma.conf.js b/karma-tests/karma.conf.js
@@ -1,8 +1,3 @@
-if (!process.env['SAUCE_USERNAME'] || !process.env['SAUCE_ACCESS_KEY']) {
-    console.error('Sauce Labs account details not set, skipping Karma tests');
-    process.exit(0);
-}
-
 var sauceLabsConfig = {
     testName: 'PyWB Client Tests',
 };
@@ -15,7 +10,7 @@ if (process.env.TRAVIS_JOB_NUMBER) {
 
 var WOMBAT_JS_PATH = 'pywb/static/wombat.js';
 
-var customLaunchers = {
+var sauceLaunchers = {
     sl_chrome: {
         base: 'SauceLabs',
         browserName: 'chrome',
@@ -26,25 +21,37 @@ var customLaunchers = {
         browserName: 'firefox',
     },
 
-/*  Safari and Edge are currently broken in
-    pywb.
-
-    See: https://github.com/ikreymer/pywb/issues/148 (Edge)
-         https://github.com/ikreymer/pywb/issues/147 (Safari)
-
     sl_safari: {
         base: 'SauceLabs',
         browserName: 'safari',
         platform: 'OS X 10.11',
         version: '9.0',
     },
+
     sl_edge: {
         base: 'SauceLabs',
         browserName: 'MicrosoftEdge',
     },
-*/
 };
 
+var localLaunchers = {
+    localFirefox: {
+        base: 'Firefox',
+    },
+};
+
+var customLaunchers = {};
+
+if (process.env['SAUCE_USERNAME'] && process.env['SAUCE_ACCESS_KEY']) {
+    customLaunchers = sauceLaunchers;
+} else {
+    console.error('Sauce Labs account details not set, ' +
+                  'Karma tests will be run only against local browsers.' +
+                  'Set SAUCE_USERNAME and SAUCE_ACCESS_KEY environment variables to ' +
+                  'run tests against Sauce Labs browsers');
+    customLaunchers = localLaunchers;
+}
+
 module.exports = function(config) {
   config.set({
     basePath: '../',
@@ -58,6 +65,11 @@ module.exports = function(config) {
         included: false,
         served: true,
       },
+      {
+        pattern: 'karma-tests/dummy.html',
+        included: false,
+        served: true,
+      },
       'karma-tests/*.spec.js',
     ],
 
@@ -75,9 +87,15 @@ module.exports = function(config) {
 
     sauceLabs: sauceLabsConfig,
 
-    // use an extended timeout for capturing Sauce Labs
-    // browsers in case the service is busy
+    // Set extended timeouts to account for the slowness
+    // in connecting to remote browsers (eg. when using
+    // Sauce Labs)
+    //
+    // See https://oligofren.wordpress.com/2014/05/27/running-karma-tests-on-browserstack/
     captureTimeout: 3 * 60000,
+    browserNoActivityTimeout: 30 * 1000,
+    browserDisconnectTimeout: 10 * 1000,
+    browserDisconnectTolerance: 1,
 
     customLaunchers: customLaunchers,
 
diff --git a/karma-tests/wombat.spec.js b/karma-tests/wombat.spec.js
@@ -1,4 +1,3 @@
-var WOMBAT_SRC = '../pywb/static/wombat.js';
 var DEFAULT_TIMEOUT = 20000;
 
 // creates a new document in an <iframe> and runs
@@ -12,7 +11,7 @@ var DEFAULT_TIMEOUT = 20000;
 function runWombatTest(testCase, done) {
     // create an <iframe>
     var testFrame = document.createElement('iframe');
-    testFrame.src = '/dummy.html';
+    testFrame.src = '/base/karma-tests/dummy.html';
     document.body.appendChild(testFrame);
 
     testFrame.contentWindow.addEventListener('load', function () {
@@ -27,8 +26,20 @@ function runWombatTest(testCase, done) {
             done(new Error(ex));
         };
 
-        // expose chai assertions to the <iframe>
-        window.assert = assert;
+        // expose utility methods for assertion testing in tests.
+        // (We used to expose chai asserts here but Karma's default
+        //  error reporter replaces URLs in exception messages with
+        //  the corresponding file paths, which is unhelpful for us
+        //  since assert.equal() will often be called with URLs in our tests)
+        window.assert = {
+            equal: function (a, b) {
+                if (a !== b) {
+                    x.equal(a, b);
+                    console.error('Mismatch between', a, 'and', b);
+                    throw new Error('AssertionError');
+                }
+            }
+        };
 
         runFunctionInIFrame(function () {
             // re-assign the iframe's console object to the parent window's
@@ -44,8 +55,21 @@ function runWombatTest(testCase, done) {
             };
 
             // expose chai's assertion testing API to the test script
-            assert = window.parent.assert;
-            reportError = window.parent.reportError;
+            window.assert = window.parent.assert;
+            window.reportError = window.parent.reportError;
+
+            // helpers which check whether DOM property overrides are supported
+            // in the current browser
+            window.domTests = {
+                areDOMPropertiesConfigurable: function () {
+                    var descriptor = Object.getOwnPropertyDescriptor(Node.prototype, 'baseURI');
+                    if (descriptor && !descriptor.configurable) {
+                        return false;
+                    } else {
+                        return true;
+                    }
+                }
+            };
         });
 
         try {
@@ -108,41 +132,66 @@ describe('WombatJS', function () {
         }, done);
     });
 
-    it('should rewrite document.baseURI', function (done) {
-        runWombatTest({
-            initScript: function () {
-                wbinfo = {
-                    wombat_opts: {},
-                    prefix: window.location.origin,
-                    wombat_ts: '',
-                };
-            },
-            wombatScript: wombatScript,
-            testScript: function () {
-                var baseURI = document.baseURI;
-                if (typeof baseURI !== 'string') {
-                    throw new Error('baseURI is not a string');
-                }
-                assert.equal(baseURI, 'http:///dummy.html');
-            },
-        }, done);
+    describe('anchor rewriting', function () {
+        it('should rewrite links in dynamically injected <a> tags', function (done) {
+            runWombatTest({
+                initScript: function () {
+                    wbinfo = {
+                        wombat_opts: {},
+                        prefix: window.location.origin,
+                        wombat_ts: '',
+                    };
+                },
+                wombatScript: wombatScript,
+                html: '<a href="foobar.html" id="link">A link</a>',
+                testScript: function () {
+                    var link = document.getElementById('link');
+                    if (domTests.areDOMPropertiesConfigurable()) {
+                        assert.equal(link.href, 'http:///base/karma-tests/foobar.html');
+                    }
+                },
+            }, done);
+        });
     });
 
-    it('should rewrite links in dynamically injected <a> tags', function (done) {
-        runWombatTest({
-            initScript: function () {
-                wbinfo = {
-                    wombat_opts: {},
-                    prefix: window.location.origin,
-                    wombat_ts: '',
-                };
-            },
-            wombatScript: wombatScript,
-            html: '<a href="foobar.html" id="link">A link</a>',
-            testScript: function () {
-                var link = document.getElementById('link');
-                assert.equal(link.href, 'http:///foobar.html');
-            },
-        }, done);
+    describe('base URL overrides', function () {
+        it('document.baseURI should return the original URL', function (done) {
+            runWombatTest({
+                initScript: function () {
+                    wbinfo = {
+                        wombat_opts: {},
+                        prefix: window.location.origin,
+                        wombat_ts: '',
+                    };
+                },
+                wombatScript: wombatScript,
+                testScript: function () {
+                    var baseURI = document.baseURI;
+                    if (typeof baseURI !== 'string') {
+                        throw new Error('baseURI is not a string');
+                    }
+                    if (domTests.areDOMPropertiesConfigurable()) {
+                        assert.equal(baseURI, 'http:///base/karma-tests/dummy.html');
+                    }
+                },
+            }, done);
+        });
+
+        it('should allow base.href to be assigned', function (done) {
+            runWombatTest({
+                initScript: function () {
+                    wbinfo = {
+                        wombat_opts: {},
+                    };
+                },
+                wombatScript: wombatScript,
+                testScript: function () {
+                    'use strict';
+                    var baseElement = document.createElement('base');
+                    baseElement.href = 'http://foobar.com/base';
+                    assert.equal(baseElement.href, 'http://foobar.com/base');
+                },
+            }, done);
+        });
     });
 });
diff --git a/pywb/static/wombat.js b/pywb/static/wombat.js
@@ -379,11 +379,16 @@ var wombat_internal = function($wbwindow) {
         }
 
         try {
-            Object.defineProperty(obj, prop, {
-                configurable: false,
-                set: set_func,
-                get: get_func
-            });
+            var descriptor = {
+                configurable: true,
+                get: get_func,
+            };
+
+            if (set_func) {
+                descriptor.set = set_func;
+            }
+
+            Object.defineProperty(obj, prop, descriptor);
 
             return true;
         } catch (e) {
@@ -762,15 +767,14 @@ var wombat_internal = function($wbwindow) {
         def_prop($wbwindow.HTMLBaseElement.prototype, "href", undefined, base_href_get);
 
         // Shared baseURI
-        var orig_getter = get_orig_getter($wbwindow.Node, "baseURI");
+        var orig_getter = get_orig_getter($wbwindow.Node.prototype, "baseURI");
         if (orig_getter) {
             var get_baseURI = function() {
                 var res = orig_getter.call(this);
                 return extract_orig(res);
             }
 
-            def_prop($wbwindow.HTMLElement.prototype, "baseURI", undefined, get_baseURI);
-            def_prop($wbwindow.HTMLDocument.prototype, "baseURI", undefined, get_baseURI);
+            def_prop($wbwindow.Node.prototype, "baseURI", undefined, get_baseURI);
         }
     }
 
