Merge pull request #499 from Polymer/polymer_issue_421_take2

Re-land fix for unload/beforeunload events
googlearchive · Sep 5, 2014 · 565b978 · 565b978
2 parents 5230708 + 55aada6
commit 565b978
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 5 deletions.
diff --git a/src/wrappers/events.js b/src/wrappers/events.js
@@ -237,6 +237,19 @@
     }
   }
 
+
+  function isLoadLikeEvent(event) {
+    switch (event.type) {
+      // http://www.whatwg.org/specs/web-apps/current-work/multipage/webappapis.html#events-and-the-window-object
+      case 'load':
+      // http://www.whatwg.org/specs/web-apps/current-work/multipage/browsers.html#unloading-documents
+      case 'beforeunload':
+      case 'unload':
+        return true;
+    }
+    return false;
+  }
+
   function dispatchEvent(event, originalWrapperTarget) {
     if (currentlyDispatchingEvents.get(event))
       throw new Error('InvalidStateError');
@@ -254,12 +267,11 @@
     // http://www.whatwg.org/specs/web-apps/current-work/multipage/the-end.html#the-end
     var overrideTarget;
     var win;
-    var type = event.type;
 
     // Should really be not cancelable too but since Firefox has a bug there
     // we skip that check.
     // https://bugzilla.mozilla.org/show_bug.cgi?id=999456
-    if (type === 'load' && !event.bubbles) {
+    if (isLoadLikeEvent(event) && !event.bubbles) {
       var doc = originalWrapperTarget;
       if (doc instanceof wrappers.Document && (win = doc.defaultView)) {
         overrideTarget = doc;
@@ -274,7 +286,7 @@
       } else {
         eventPath = getEventPath(originalWrapperTarget, event);
 
-        if (event.type !== 'load') {
+        if (!isLoadLikeEvent(event)) {
           var doc = eventPath[eventPath.length - 1];
           if (doc instanceof wrappers.Document)
             win = doc.defaultView;
@@ -380,7 +392,6 @@
     var type = event.type;
 
     var anyRemoved = false;
-    // targetTable.set(event, target);
     targetTable.set(event, target);
     currentTargetTable.set(event, currentTarget);
 
@@ -465,7 +476,12 @@
   function Event(type, options) {
     if (type instanceof OriginalEvent) {
       var impl = type;
-      if (!OriginalBeforeUnloadEvent && impl.type === 'beforeunload') {
+      // In browsers that do not correctly support BeforeUnloadEvent we get to
+      // the generic Event wrapper but we still want to ensure we create a
+      // BeforeUnloadEvent. Since BeforeUnloadEvent calls super, we need to
+      // prevent reentrancty.
+      if (!OriginalBeforeUnloadEvent && impl.type === 'beforeunload' &&
+          !(this instanceof BeforeUnloadEvent)) {
         return new BeforeUnloadEvent(impl);
       }
       setWrapper(impl, this);

diff --git a/test/html/on-unload-test.html b/test/html/on-unload-test.html
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<script src="../../../tools/test/chai/chai.js"></script>
+<script src="../../../tools/test/htmltest.js"></script>
+<script src="../../shadowdom.js"></script>
+<script>
+// Note: this test will navigate away from the page. It is designed to be run
+// in an iframe. Use ShadowDOM/test/runner.html?grep=unload for running it by
+// itself.
+
+function runTest() {
+  var assert = chai.assert;
+  var doc = ShadowDOMPolyfill.wrap(document);
+  var win = ShadowDOMPolyfill.wrap(window);
+
+  // Note: IE gives `window` as the target for beforeunload/unload. Others give
+  // document. It's not clear than anyone gets it right, my reading of the
+  // current spec is target should be window for beforeunload, but is overridden
+  // to be document for unload. Both events are dispatched on window:
+  // http://www.whatwg.org/specs/web-apps/current-work/multipage/browsers.html#unloading-documents
+  var expectedTarget = /Trident/.test(navigator.userAgent) ? win : doc;
+
+  var beforeunloadCalled = 0;
+  window.addEventListener('beforeunload', function(e) {
+    beforeunloadCalled++;
+    assert.equal(e.target, expectedTarget);
+  });
+
+  window.addEventListener('unload', function(e) {
+    assert.equal(beforeunloadCalled, 1);
+    assert.equal(e.target, expectedTarget);
+    done();
+  });
+
+  location.href = 'about:blank';
+}
+
+document.addEventListener('DOMContentLoaded', function(e) {
+  // Note: setTimeout makes IE happy. If document.readyState == 'interactive'
+  // at the time we run the tests, IE won't fire an unload in the iframe.
+  setTimeout(runTest, 0);
+});
+
+</script>
diff --git a/test/js/events.js b/test/js/events.js
@@ -928,6 +928,7 @@ test('retarget order (multiple shadow roots)', function() {
   });
 
   htmlTest('html/on-load-test.html');
+  htmlTest('html/on-unload-test.html');
 
   test('event wrap round trip', function() {
     var e = new Event('x');