From 1b02e96e77a0ef201976341fade763c617eca0d5 Mon Sep 17 00:00:00 2001
From: Herr Kaste <thdz.x@gmx.net>
Date: Wed, 6 Apr 2016 01:55:31 +0200
Subject: [PATCH] Compute and use correct annotation value during config

* Refactor _applyEffectValue

... to get a new function that computes the final value an annotation
effect will propagate downwards.

* Compute correct annotation value during config.

Due to a typo compound annotations were never actually excluded from
config, they only received the wrong value; remove that false conditional
b/c with this change the compounds don't have to be excluded at all.
---
 src/lib/bind/effects.html         | 11 +-------
 src/standard/configure.html       | 10 ++++---
 src/standard/effectBuilder.html   | 44 ++++++++++++++++++++++---------
 test/unit/configure-elements.html | 23 +++++++++++++++-
 test/unit/configure.html          | 13 +++++++++
 5 files changed, 73 insertions(+), 28 deletions(-)

diff --git a/src/lib/bind/effects.html b/src/lib/bind/effects.html
index b8d0f4cf50..d2dadc82b7 100644
--- a/src/lib/bind/effects.html
+++ b/src/lib/bind/effects.html
@@ -25,13 +25,7 @@
         value = this._get(effect.value);
         this.__data__[effect.value] = value;
       }
-      var calc = effect.negate ? !value : value;
-      // For better interop, dirty check before setting when custom events
-      // are used, since the target element may not dirty check (e.g. <input>)
-      if (!effect.customEvent ||
-          this._nodes[effect.index][effect.name] !== calc) {
-        return this._applyEffectValue(effect, calc);
-      }
+      this._applyEffectValue(effect, value);
     },
 
     _reflectEffect: function(source, value, effect) {
@@ -101,9 +95,6 @@
         var args = Polymer.Bind._marshalArgs(this.__data__, effect, source, value);
         if (args) {
           var computedvalue = fn.apply(computedHost, args);
-          if (effect.negate) {
-            computedvalue = !computedvalue;
-          }
           this._applyEffectValue(effect, computedvalue);
         }
       } else if (effect.dynamicFn) {
diff --git a/src/standard/configure.html b/src/standard/configure.html
index c492bdeac7..0d496e9de8 100644
--- a/src/standard/configure.html
+++ b/src/standard/configure.html
@@ -143,18 +143,20 @@
           var fx = fx$[p];
           if (fx) {
             for (var i=0, l=fx.length, x; (i<l) && (x=fx[i]); i++) {
-              // TODO(kschaaf): compound bindings (as well as computed effects)
-              // are excluded from top-down configure for now; to be revisited
-              if (x.kind === 'annotation' && !x.isCompound) {
+              // TODO(kschaaf): computed annotations are excluded from top-down
+              // configure for now; to be revisited
+              if (x.kind === 'annotation') {
                 var node = this._nodes[x.effect.index];
                 var name = x.effect.propertyName;
                 // seeding configuration only
                 var isAttr = (x.effect.kind == 'attribute');
-                var hasEffect = (node._propertyEffects && 
+                var hasEffect = (node._propertyEffects &&
                   node._propertyEffects[name]);
                 if (node._configValue && (hasEffect || !isAttr)) {
                   var value = (p === x.effect.value) ? config[p] :
                     this._get(x.effect.value, config);
+                  value = this._computeFinalAnnotationValue(node, name, value,
+                                                            x.effect);
                   if (isAttr) {
                     // For attribute bindings, flow through the same ser/deser
                     // process to ensure the value is the same as if it were
diff --git a/src/standard/effectBuilder.html b/src/standard/effectBuilder.html
index a9b693eaa9..8c0809c8ea 100644
--- a/src/standard/effectBuilder.html
+++ b/src/standard/effectBuilder.html
@@ -322,16 +322,41 @@
     _applyEffectValue: function(info, value) {
       var node = this._nodes[info.index];
       var property = info.name;
+
+      value = this._computeFinalAnnotationValue(node, property, value, info);
+
+      // For better interop, dirty check before setting when custom events
+      // are used, since the target element may not dirty check (e.g. <input>)
+      if (info.customEvent && node[property] === value) {
+        return;
+      }
+
+      if (info.kind == 'attribute') {
+        this.serializeValueToAttribute(value, property, node);
+      } else {
+        var pinfo = node._propertyInfo && node._propertyInfo[property];
+        if (pinfo && pinfo.readOnly) {
+          return;
+        }
+        // Ideally we would call setProperty using fromAbove: true to avoid
+        // spinning the wheel needlessly, but we found that users were listening
+        // for change events outside of bindings
+        this.__setProperty(property, value, false, node);
+      }
+    },
+
+    _computeFinalAnnotationValue: function(node, property, value, info) {
+      if (info.negate) {
+        value = !value;
+      }
+
       if (info.isCompound) {
         var storage = node.__compoundStorage__[property];
         storage[info.compoundIndex] = value;
         value = storage.join('');
       }
-      // special processing for 'class' and 'className'; 'class' handled
-      // when attr is serialized.
-      if (info.kind == 'attribute') {
-        this.serializeValueToAttribute(value, property, node);
-      } else {
+
+      if (info.kind !== 'attribute') {
         // TODO(sorvell): consider pre-processing the following two string
         // comparisons in the hot path so this can be a boolean check
         if (property === 'className') {
@@ -342,15 +367,8 @@
             (node.localName == 'input' && property == 'value')) {
           value = value == undefined ? '' : value;
         }
-        // Ideally we would call setProperty using fromAbove: true to avoid
-        // spinning the wheel needlessly, but we found that users were listening
-        // for change events outside of bindings
-        var pinfo;
-        if (!node._propertyInfo || !(pinfo = node._propertyInfo[property]) ||
-          !pinfo.readOnly) {
-          this.__setProperty(property, value, false, node);
-        }
       }
+      return value;
     },
 
     _executeStaticEffects: function() {
diff --git a/test/unit/configure-elements.html b/test/unit/configure-elements.html
index a56bdafa4f..be40fcd34f 100644
--- a/test/unit/configure-elements.html
+++ b/test/unit/configure-elements.html
@@ -107,6 +107,16 @@
           observer: 'contentChanged',
           value: 'child'
         },
+        negatedContent: {
+          type: Boolean,
+          observer: 'negatedContentChanged',
+          value: true
+        },
+        compoundInput: {
+          type: String,
+          observer: 'compoundInputChanged',
+          value: 'default'
+        },
         object: {
           type: Object,
           notify: true,
@@ -140,6 +150,8 @@
         this.attrDashChanged = sinon.spy();
         this.attrNumberChanged = sinon.spy();
         this.attrBooleanChanged = sinon.spy();
+        this.negatedContentChanged = sinon.spy();
+        this.compoundInputChanged = sinon.spy();
       }
 
     });
@@ -166,7 +178,16 @@
 
 <dom-module id="x-configure-host">
   <template>
-    <x-configure-child id="child" content="{{content}}" object="{{object.goo}}" attr$="{{attrValue}}" attr-dash$="{{attrValue}}" attr-number$="{{attrNumber}}" attr-boolean$="{{attrBoolean}}"></x-configure-child>
+    <x-configure-child id="child"
+        content="{{content}}"
+        negated-content="[[!content]]"
+        compound-input="a [[simple]] [[content]]"
+        object="{{object.goo}}"
+        attr$="{{attrValue}}"
+        attr-dash$="{{attrValue}}"
+        attr-number$="{{attrNumber}}"
+        attr-boolean$="{{attrBoolean}}"
+    ></x-configure-child>
     <x-configure-simple-child id="simple" noeffect="{{simple}}"></x-configure-simple-child>
   </template>
   <script>
diff --git a/test/unit/configure.html b/test/unit/configure.html
index 29ecb5dffe..c55abb9e9e 100644
--- a/test/unit/configure.html
+++ b/test/unit/configure.html
@@ -66,6 +66,19 @@
       testConfigureHost(e, 'host');
     });
 
+    test('negated value configured correctly', function() {
+      var e = document.querySelector('x-configure-host');
+      assert.equal(e.$.child.negatedContent, false);
+      assert.isTrue(e.$.child.negatedContentChanged.calledOnce, 'negated content not changed exactly once');
+    });
+
+    test('compound effect resulting value set once', function() {
+      var e = document.querySelector('x-configure-host');
+
+      assert.equal(e.$.child.compoundInput, 'a simple host');
+      assert.isTrue(e.$.child.compoundInputChanged.calledOnce, 'compound content not changed exactly once');
+    });
+
     test('attribute overrides configured values and propagates', function() {
       var e = document.querySelector('x-configure-host[content]');
       testConfigureHost(e, 'attr');