Fix bug in polymer expressions where input-value bindings are incorrectly

Fix bug in polymer expressions where input-value bindings are incorrectly
updated twice, losing the cursor position.

BUG=
R=jmesserly@google.com

Committed: https://code.google.com/p/dart/source/detail?r=28480

[Siggi Cherem (dart-lang), 2013-10-09 03:25:54]

https://codereview.chromium.org/26649002/diff/1/pkg/polymer_expressions/lib/p...
File pkg/polymer_expressions/lib/polymer_expressions.dart (left):

https://codereview.chromium.org/26649002/diff/1/pkg/polymer_expressions/lib/p...
pkg/polymer_expressions/lib/polymer_expressions.dart:127: notifyChange(new
PropertyChangeRecord(_VALUE));
to clarify:
- a change in the input element will update the binding here
- assign will update _expr
- expr.onUpdate will trigger calling _setValue above
- which will notify about the change in line 119 for a second time.

[Siggi Cherem (dart-lang), 2013-10-09 03:27:00]

still pending to add some tests, just wanted to make sure this seems like a
reasonable fix

[Jennifer Messerly, 2013-10-09 05:24:52]

On 2013/10/09 03:27:00, Siggi Cherem (dart-lang) wrote:
> still pending to add some tests, just wanted to make sure this seems like a
> reasonable fix

Wow, good catch!

Could this be fixed by using notifyProperyChange and only signaling a Chang if
the value is different?

[justinfagnani, 2013-10-09 19:00:58]

https://codereview.chromium.org/26649002/diff/1/pkg/polymer_expressions/lib/p...
File pkg/polymer_expressions/lib/polymer_expressions.dart (left):

https://codereview.chromium.org/26649002/diff/1/pkg/polymer_expressions/lib/p...
pkg/polymer_expressions/lib/polymer_expressions.dart:127: notifyChange(new
PropertyChangeRecord(_VALUE));
On 2013/10/09 03:25:54, Siggi Cherem (dart-lang) wrote:
> to clarify:
> - a change in the input element will update the binding here
> - assign will update _expr
> - expr.onUpdate will trigger calling _setValue above
> - which will notify about the change in line 119 for a second time.

I think I tried removing this and ran into other issues. Can you try the large
example and see if other uses of the value bound here update?

[Jennifer Messerly, 2013-10-09 19:04:37]

https://codereview.chromium.org/26649002/diff/1/pkg/polymer_expressions/lib/p...
File pkg/polymer_expressions/lib/polymer_expressions.dart (left):

https://codereview.chromium.org/26649002/diff/1/pkg/polymer_expressions/lib/p...
pkg/polymer_expressions/lib/polymer_expressions.dart:127: notifyChange(new
PropertyChangeRecord(_VALUE));
On 2013/10/09 19:00:59, justinfagnani wrote:
> On 2013/10/09 03:25:54, Siggi Cherem (dart-lang) wrote:
> > to clarify:
> > - a change in the input element will update the binding here
> > - assign will update _expr
> > - expr.onUpdate will trigger calling _setValue above
> > - which will notify about the change in line 119 for a second time.
> 
> I think I tried removing this and ran into other issues. Can you try the large
> example and see if other uses of the value bound here update? 

probably the right fix is:

try {
  var oldValue = _value;
  assign(...);
  notifyPropertyChange(#value, oldValue, _value);
} ...

ditto for the other notifyChange above :)

[Siggi Cherem (dart-lang), 2013-10-10 00:49:47]

ptal - I added a test for it too, woo hoo!

https://codereview.chromium.org/26649002/diff/8001/pkg/polymer_expressions/te...
File pkg/polymer_expressions/test/bindings_test.dart (right):

https://codereview.chromium.org/26649002/diff/8001/pkg/polymer_expressions/te...
pkg/polymer_expressions/test/bindings_test.dart:68: expect(el.selectionStart,
4);
without the fix, this returns 6.

[Jennifer Messerly, 2013-10-10 21:15:01]

lgtm

[Siggi Cherem (dart-lang), 2013-10-10 21:29:13]

Committed patchset #2 manually as r28480 (presubmit successful).