Revision: 5441
Author:   [email protected]
Date:     Fri Jun  7 13:30:31 2013
Log:      Fix syntax and escaping of window.location properties.
https://codereview.appspot.com/9981045

Fixes to window.location per MDN and empirical testing w/ Chrome:
* window.location.{pathname,search,hash} are in escaped form.
* window.location.host correct when no port number is present.
* window.location.pathname is never empty, but at least '/'.
* window.location.{search,hash} include the ? or #.
* Fixed cases where 'nosuchhost.invalid' placeholder values occur
  even though there is a proper value, which happens to be the empty
  string.

Supporting changes:
* Move testLocation from test-domado-dom-guest to test-domado-global and
  run it on several different URLs.
* Refactor test-domado-global to introduce registerGlobalTest which can
  handle more cases than registerGuestTest.

[email protected]


http://code.google.com/p/google-caja/source/detail?r=5441

Added:
 /trunk/tests/com/google/caja/plugin/es53-test-domado-global-location.js
Modified:
 /trunk/src/com/google/caja/plugin/domado.js
 /trunk/tests/com/google/caja/plugin/es53-test-domado-dom-guest.html
 /trunk/tests/com/google/caja/plugin/es53-test-domado-global.js

=======================================
--- /dev/null
+++ /trunk/tests/com/google/caja/plugin/es53-test-domado-global-location.js Fri Jun 7 13:30:31 2013
@@ -0,0 +1,56 @@
+function testLocation(beSpecific) {
+  var assembledHost = window.location.port === ''
+      ? window.location.hostname
+      : window.location.hostname + ':' + window.location.port;
+
+  assertEquals(
+      'host consistent',
+      assembledHost,
+      window.location.host);
+  assertEquals(
+      'href consistent',
+ window.location.protocol + '//' + assembledHost + window.location.pathname
+          + window.location.search + window.location.hash,
+      window.location.href);
+  // TODO(jasvir): Uncomment after wiring up document.domain
+  // by untangling the cyclic dependence between
+  // TameWindow and TameDocument
+  //    assertEquals(
+  //        window.location.hostname,
+  //        document.domain);
+
+ // Check that the right answers are given for a specific known URL (whereas
+  // the other asserts are only that we are internally consistent).
+  if (beSpecific) {
+ // (The specific URL shown here is left over from the old location of this
+    // test and could perfectly well be changed to something else.)
+    assertEquals(
+        'href specific',
+        'http://' + window.location.hostname
+        + ':' + window.location.port
+ + '/ant-testlib/com/google/caja/plugin/es53-test-domado-dom-guest.html',
+        window.location.href);
+    assertEquals(
+        'hash',
+        '',
+        window.location.hash);
+    assertEquals(
+        'pathname specific',
+        '/ant-testlib/com/google/caja/plugin/es53-test-domado-dom-guest.html',
+        window.location.pathname);
+    assertTrue(
+        'port',
+        /^[1-9][0-9]+$/.test(window.location.port));
+    assertEquals(
+        'protocol',
+        'http:',
+        window.location.protocol);
+    assertEquals(
+        'search',
+        '',
+        window.location.search);
+  }
+
+  // document.location is an identical property to window.location
+  assertTrue('document.location', window.location === document.location);
+}
=======================================
--- /trunk/src/com/google/caja/plugin/domado.js Tue May 28 14:58:38 2013
+++ /trunk/src/com/google/caja/plugin/domado.js Fri Jun  7 13:30:31 2013
@@ -6158,17 +6158,21 @@
         var parsed = URI.parse(base);
         var host = null;
         if (parsed.hasDomain()) {
- host = parsed.hasPort() ? parsed.getDomain() + ':' + parsed.getPort() : null; + host = parsed.hasPort() ? parsed.getDomain() + ':' + parsed.getPort()
+              : parsed.getDomain();
+        }
+        if (!parsed.hasPath()) {
+          parsed.setRawPath('/');
         }
         domicile.pseudoLocation = {
           href: parsed.toString(),
-          hash: parsed.getFragment(),
+          hash: parsed.hasFragment() ? '#' + parsed.getRawFragment() : '',
           host: host,
           hostname: parsed.getDomain(),
-          port: parsed.getPort(),
+          port: parsed.hasPort() ? parsed.getPort() : '',
           protocol: parsed.hasScheme() ? parsed.getScheme() + ':' : null,
-          pathname: parsed.getPath(),
-          search: parsed.getQuery()
+          pathname: parsed.getRawPath(),
+          search: parsed.hasQuery() ? '?' + parsed.getRawQuery() : ''
         };
       });

@@ -6613,7 +6617,8 @@
             enumerable: true,
             get: innocuous(function() {
               try {
-                return String(domicile.pseudoLocation[f] || dflt);
+                var v = domicile.pseudoLocation[f];
+                return String(v !== null ? v : dflt);
               } catch (e) {
// paranoia - domicile.pseudoLocation is potentially replaceable // by the host. TODO(kpreid): put pseudoLocation somewhere not
=======================================
--- /trunk/tests/com/google/caja/plugin/es53-test-domado-dom-guest.html Tue May 28 14:58:38 2013 +++ /trunk/tests/com/google/caja/plugin/es53-test-domado-dom-guest.html Fri Jun 7 13:30:31 2013
@@ -2990,58 +2990,6 @@
   });
 </script>

-<div id="testLocation" class="testcontainer">
-  testLocation
-</div>
-<script type="text/javascript">
-  jsunitRegister('testLocation',
-                 function testLocation() {
-    assertEquals(
-        window.location.host,
-        window.location.hostname + ':' + window.location.port);
-    assertEquals(
-        window.location.href,
-        window.location.protocol + '//' + window.location.hostname
-        + ':' + window.location.port + window.location.pathname
-        + window.location.search + window.location.hash);
-    assertEquals(
-        'http://' + window.location.hostname
-        + ':' + window.location.port
-        + '/ant-testlib/com/google/caja/plugin/'
-        + 'es53-test-domado-dom-guest.html',
-        window.location.href);
-    assertEquals(
-        '',
-        window.location.hash);
-    assertEquals(
-        window.location.hostname + ':' + window.location.port,
-        window.location.host);
-    assertEquals(''
-        + '/ant-testlib/com/google/caja/plugin/'
-        + 'es53-test-domado-dom-guest.html',
-        window.location.pathname);
-    assertTrue(
-        /^[1-9][0-9]+$/.test(window.location.port));
-    assertEquals(
-        'http:',
-        window.location.protocol);
-    assertEquals(
-        '',
-        window.location.search);
-    // TODO(jasvir): Uncomment after wiring up document.domain
-    // by untangling the cyclic dependence between
-    // TameWindow and TameDocument
-    //    assertEquals(
-    //        window.location.hostname,
-    //        document.domain);
-
-    // document.location is an identical property to window.location
-    assertTrue('document.location', window.location === document.location);
-
-    pass('testLocation');
-  });
-</script>
-
 <div id="testNavigator" class="testcontainer">testNavigator </div>
 <script type="text/javascript">
   jsunitRegister('testNavigator',
=======================================
--- /trunk/tests/com/google/caja/plugin/es53-test-domado-global.js Wed May 22 14:02:57 2013 +++ /trunk/tests/com/google/caja/plugin/es53-test-domado-global.js Fri Jun 7 13:30:31 2013
@@ -26,35 +26,51 @@
     forceES5Mode: inES5Mode
   });

-  function registerGuestTest(testName, html, varargs) {
-    var guestTestArgs = Array.prototype.slice.call(arguments, 2);
-
-    jsunitRegister(testName,
-        function guestTestWrapper() {
+  function registerGlobalTest(cond, testName, url, html, callback) {
+    jsunitRegisterIf(cond, testName, function globalTestWrapper() {
       var div = createDiv();
-      caja.load(div, undefined, function (frame) {
-        frame.code(
-            location.protocol + '//' + location.host + '/',
-            'text/html',
-            html)
+      // Load callback is NOT a jsunitCallback because that would be mostly
+      // noise.
+      caja.load(div, undefined, function(frame) {
+        frame.code(url, 'text/html', html)
           .run(createExtraImportsForTesting(caja, frame),
-              jsunitCallback(function(result) {
-                // Domado delays onload handlers with setTimeout(,0),
-                // so we have to delay globalGuestTest to make sure
-                // it's run after all the onloads fire.
-                window.setTimeout(function() {
-                  var tameGT = frame.imports.globalGuestTest;
-                  assertEquals('typeof globalGuestTest', 'function',
-                      typeof tameGT);
-                  frame.untame(tameGT).apply(undefined, guestTestArgs);
-                  jsunitPass(testName);
-                }, 0);
+              jsunitCallback(function() {
+                callback(frame);
               }, testName, frame));
       });
     });
   }

-  fetch('es53-test-domado-global-html-guest.js', function (htmlGuestJs) {
+  function registerGuestTest(testName, html, varargs) {
+    var guestTestArgs = Array.prototype.slice.call(arguments, 2);
+    registerGlobalTest(
+        true,
+        testName,
+        location.protocol + '//' + location.host + '/',
+        html,
+        function(frame) {
+          // Domado delays onload handlers with setTimeout(,0),
+          // so we have to delay globalGuestTest to make sure
+          // it's run after all the onloads fire.
+          window.setTimeout(function() {
+            var tameGT = frame.imports.globalGuestTest;
+            assertEquals('typeof globalGuestTest', 'function',
+                typeof tameGT);
+            frame.untame(tameGT).apply(undefined, guestTestArgs);
+            jsunitPass(testName);
+          }, 0);
+        });
+  }
+
+  function fetches(callback) {
+    fetch('es53-test-domado-global-html-guest.js', function(htmlGuestJs) {
+      fetch('es53-test-domado-global-location.js', function(locationJs) {
+        callback(htmlGuestJs, locationJs);
+      });
+    });
+  }
+
+  fetches(function(htmlGuestJs, locationJs) {
     /**
      * Tests of how Caja handles omitted structure in HTML document inputs.
      */
@@ -99,24 +115,15 @@
           '', 'b$');

       // Test that a completely empty document still produces structure.
-      // TODO(kpreid): refactor registerGuestTest so this can be shorter.
-      jsunitRegister('testEmptyInput',
-          function testEmptyInput() {
-        var div = createDiv();
-        caja.load(div, undefined, function (frame) {
-          frame.code(
-              location.protocol + '//' + location.host + '/',
-              'text/html',
-              '')
-            .run(createExtraImportsForTesting(caja, frame),
-                jsunitCallback(function(result) {
- var guestHtml = ('<html><head></head><body></body></html>' - .replace(/<\/?/g, function(m) { return m + 'caja-v-'; }));
-                  assertEquals(guestHtml, frame.innerContainer.innerHTML);
-                  jsunitPass('testEmptyInput');
-                }, 'testEmptyInput', frame));
-        });
-      });
+      registerGlobalTest(true, 'testEmptyInput',
+          location.protocol + '//' + location.host + '/',
+          '',
+          function(frame) {
+            var guestHtml = ('<html><head></head><body></body></html>'
+                .replace(/<\/?/g, function(m) { return m + 'caja-v-'; }));
+            assertEquals(guestHtml, frame.innerContainer.innerHTML);
+            jsunitPass('testEmptyInput');
+          });

       registerStructureTest('testEmptyVirtualizedElementInHead',
           // Regression test for <body> getting embedded in <head> due to
@@ -271,19 +278,13 @@
      * Issue 1589, html-emitter gets confused in a couple ways when
      * head has script but body doesn't
      */
-    jsunitRegisterIf(!inES5Mode, 'testHtmlEmitterFinishInHead',
-                   function testHtmlEmitterFinishInHead() {
-      var div = createDiv();
-      caja.load(div, undefined, function(frame) {
-        var imports = createExtraImportsForTesting(caja, frame);
-        frame.api(imports);
-        frame.code(
-          'http://nonexistent/', 'text/html',
-          // note, the space is necessary to trigger the bug
-          '<html><head> <script>;</script></head>'
-            + '<body><div>2</div></body></html>');
-        frame.run(function(result) {
-          var doc = div.firstChild.firstChild;
+    registerGlobalTest(!inES5Mode, 'testHtmlEmitterFinishInHead',
+        'http://nonexistent/',
+        // note, the space is necessary to trigger the bug
+        '<html><head> <script>;</script></head>'
+            + '<body><div>2</div></body></html>',
+        function(frame) {
+          var doc = frame.div.firstChild.firstChild;
           var html = canonInnerHtml(doc.innerHTML);
           assertEquals(
             '<caja-v-html>'
@@ -293,8 +294,39 @@
             html);
           jsunitPass('testHtmlEmitterFinishInHead');
         });
-      });
-    });
+
+    /**
+     * Tests of window.location.
+     */
+    (function() {
+      function registerLocationTest(tag, url, opt_specific) {
+        var testName = 'testLocation-' + tag;
+        registerGlobalTest(true, testName,
+            url,
+            '<script>' + locationJs + '</script>',
+            function(frame) {
+              frame.untame(frame.imports.testLocation)(opt_specific);
+              jsunitPass(testName);
+            });
+      }
+
+      registerLocationTest('original',
+          'http://localhost:8000/ant-testlib/com/google/caja/plugin/'
+          + 'es53-test-domado-dom-guest.html',
+          true);
+
+      registerLocationTest('noPath',
+          'https://nopath.test');
+
+      registerLocationTest('path',
+          'http://path.test/foo/bar%2Fbaz');
+
+      registerLocationTest('search',
+          'http://search.test/foo?bar=ba%26z');
+
+      registerLocationTest('hash',
+          'http://hash.test/foo#bar%23baz');
+    })();

     readyToTest();
     jsunitRun();

--

--- You received this message because you are subscribed to the Google Groups "Google Caja Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to