Fixes #4550. Ensure that detached cannot run before an element is “readied.” This fixes an issue that allowed an element with disable-upgrade to process the detached callback.

Author: Steven Orvell

src/mini/ready.html

@@ -32,6 +32,7 @@
 (function() {
 
   var baseAttachedCallback = Polymer.Base.attachedCallback;
+  var baseDetachedCallback = Polymer.Base.detachedCallback;
 
   Polymer.Base._addFeature({
 
@@ -152,6 +153,11 @@
         this._attachedPending = false;
         this.attachedCallback();
       }
+      // only call detached if the element is actually detached
+      if (this._detachedPending && !Polymer.dom(document.body).deepContains(this)) {
+        this._attachedPending = false;
+        this.detachedCallback();
+      }
     },
 
     // for system overriding
@@ -175,8 +181,27 @@
       } else {
         this._attachedPending = true;
       }
+    },
+
+    /**
+     * Polymer library implementation of the Custom Elements `detachedCallback`.
+     *
+     * Note, users should not override `detachedCallback`, and instead should
+     * implement the `detached` method on Polymer elements to receive
+     * detached-time callbacks.
+     *
+     * @protected
+     */
+    detachedCallback: function() {
+      if (this._readied) {
+        baseDetachedCallback.call(this);
+      } else {
+        this._detachedPending = true;
+      }
     }
 
+
+
   });
 
 })();

test/unit/element-disable-upgrade.html

@@ -32,12 +32,42 @@
   </script>
 </dom-module>
 
+<dom-module id="x-attach">
+  <template>
+    <style>
+    :host {
+      display: block;
+    }
+    </style>
+    <div id="child">Live!</div>
+  </template>
+  <script>
+  HTMLImports.whenReady(function() {
+    Polymer({
+      is: 'x-attach',
+      attached: function() {
+        this._wasAttached = true;
+      },
+      detached: function() {
+        this._wasDetached = true;
+      }
+    });
+  });
+  </script>
+</dom-module>
+
 <test-fixture id="simple">
   <template>
     <x-lazy disable-upgrade></x-lazy>
   </template>
 </test-fixture>
 
+<test-fixture id="attach">
+  <template>
+    <x-attach disable-upgrade></x-attach>
+  </template>
+</test-fixture>
+
 <dom-module id="x-complicated-child">
   <script>
   HTMLImports.whenReady(function() {
@@ -141,6 +171,40 @@
       assert.ok(el.$.child);
     });
   });
+  suite('disableUpgrade attach/detach', function() {
+    var el;
+    setup(function() {
+      el = fixture('attach');
+    });
+    test('attached does not fire when element is not yet enabled', function() {
+      assert.notOk(el._wasAttached);
+      el.removeAttribute('disable-upgrade');
+      assert.ok(el._wasAttached);
+    });
+    test('detached does not fire when element is not yet enabled', function() {
+      el.parentNode.removeChild(el);
+      Polymer.dom.flush();
+      assert.notOk(el._wasAttached);
+      assert.notOk(el._wasDetached);
+      el.removeAttribute('disable-upgrade');
+      assert.ok(el._wasAttached);
+      assert.ok(el._wasDetached);
+    });
+    test('detached does not fire when element is detached/attached when not yet enabled', function() {
+      var parent = el.parentNode;
+      parent.removeChild(el);
+      Polymer.dom.flush();
+      assert.notOk(el._wasAttached);
+      assert.notOk(el._wasDetached);
+      parent.appendChild(el);
+      Polymer.dom.flush();
+      assert.notOk(el._wasAttached);
+      assert.notOk(el._wasDetached);
+      el.removeAttribute('disable-upgrade');
+      assert.ok(el._wasAttached);
+      assert.notOk(el._wasDetached);
+    });
+  });
   suite('disableUpgrade and Databinding', function() {
     test('binding to disable-upgrade with true', function() {
       var el = fixture('databind-lazy');