Merge pull request #5059 from jdfreder/widgets-patch-fix

Fix incorrect `Patch` logic in widget code

IPython/html/static/notebook/js/widgets/widget.js

@@ -32,6 +32,7 b' function(WidgetManager, _, Backbone){'
             //      An ID unique to this model.
             // comm : Comm instance (optional)
             this.widget_manager = widget_manager;
+            this._buffered_state_diff = {};
             this.pending_msgs = 0;
             this.msg_throttle = 3;
             this.msg_buffer = null;
@@ -93,7 +94,7 b' function(WidgetManager, _, Backbone){'
             _.each(state, function(value, key) {
                 that.key_value_lock = [key, value];
                 try {
-                    that.set(key, that._unpack_models(value));
+                    WidgetModel.__super__.set.apply(that, [key, that._unpack_models(value)]);
                 } finally {
                     that.key_value_lock = null;
                 }
@@ -135,6 +136,17 b' function(WidgetManager, _, Backbone){'
             return callbacks;
         },
 
+        set: function(key, val, options) {
+            // Set a value.
+            var return_value = WidgetModel.__super__.set.apply(this, arguments);
+
+            // Backbone only remembers the diff of the most recent set()
+            // operation.  Calling set multiple times in a row results in a 
+            // loss of diff information.  Here we keep our own running diff.
+            this._buffered_state_diff = $.extend(this._buffered_state_diff, this.changedAttributes() || {});
+            return return_value;
+        },
+
         sync: function (method, model, options) {
             // Handle sync to the back-end.  Called when a model.save() is called.
 
@@ -158,6 +170,7 b' function(WidgetManager, _, Backbone){'
             }
 
             // Only sync if there are attributes to send to the back-end.
+            attrs = this._pack_models(attrs);
             if (_.size(attrs) > 0) {
 
                 // If this message was sent via backbone itself, it will not
@@ -197,13 +210,14 b' function(WidgetManager, _, Backbone){'
             // Since the comm is a one-way communication, assume the message 
             // arrived.  Don't call success since we don't have a model back from the server
             // this means we miss out on the 'sync' event.
+            this._buffered_state_diff = {};
         },
 
         save_changes: function(callbacks) {
             // Push this model's state to the back-end
             //
             // This invokes a Backbone.Sync.
-            this.save(this.changedAttributes(), {patch: true, callbacks: callbacks});
+            this.save(this._buffered_state_diff, {patch: true, callbacks: callbacks});
         },
 
         _pack_models: function(value) {

IPython/html/tests/widgets/widget.js

@@ -8,13 +8,14 b' var recursive_compare = function(a, b) {'
 
     if (same) {
         if (a instanceof Object) {
-            for (var key in a) {
+            var key;
+            for (key in a) {
                 if (a.hasOwnProperty(key) && !recursive_compare(a[key], b[key])) {
                     same = false;
                     break;
                 }
             }
-            for (var key in b) {
+            for (key in b) {
                 if (b.hasOwnProperty(key) && !recursive_compare(a[key], b[key])) {
                     same = false;
                     break;
@@ -26,7 +27,7 b' var recursive_compare = function(a, b) {'
     }
     
     return same;
-}
+};
 
 // Test the widget framework.
 casper.notebook_test(function () {
@@ -58,7 +59,6 b' casper.notebook_test(function () {'
             var output = that.evaluate(function(input) {
                 var model = new IPython.WidgetModel(IPython.notebook.kernel.widget_manager, undefined);
                 var results = model._pack_models(input);
-                delete model;
                 return results;
             }, {input: input});
             that.test.assert(recursive_compare(input, output), 
@@ -68,7 +68,6 b' casper.notebook_test(function () {'
             var output = that.evaluate(function(input) {
                 var model = new IPython.WidgetModel(IPython.notebook.kernel.widget_manager, undefined);
                 var results = model._unpack_models(input);
-                delete model;
                 return results;
             }, {input: input});
             that.test.assert(recursive_compare(input, output), 
@@ -79,15 +78,64 b' casper.notebook_test(function () {'
             test_unpack(input);
         };
         
-        test_packing({0: 'hi', 1: 'bye'})
-        test_packing(['hi', 'bye'])
-        test_packing(['hi', 5])
-        test_packing(['hi', '5'])
-        test_packing([1.0, 0])
-        test_packing([1.0, false])
-        test_packing([1, false])
-        test_packing([1, false, {a: 'hi'}])
-        test_packing([1, false, ['hi']])
+        test_packing({0: 'hi', 1: 'bye'});
+        test_packing(['hi', 'bye']);
+        test_packing(['hi', 5]);
+        test_packing(['hi', '5']);
+        test_packing([1.0, 0]);
+        test_packing([1.0, false]);
+        test_packing([1, false]);
+        test_packing([1, false, {a: 'hi'}]);
+        test_packing([1, false, ['hi']]);
+
+        // Test multi-set, single touch code.  First create a custom widget.
+        this.evaluate(function() {
+            var MultiSetView = IPython.DOMWidgetView.extend({
+                render: function(){
+                    this.model.set('a', 1);
+                    this.model.set('b', 2);
+                    this.model.set('c', 3);
+                    this.touch();
+                },
+            });
+            IPython.WidgetManager.register_widget_view('MultiSetView', MultiSetView);
+        }, {});
+    });
+
+    // Try creating the multiset widget, verify that sets the values correctly.
+    var multiset = {};
+    multiset.index = this.append_cell(
+        'from IPython.utils.traitlets import Unicode, CInt\n' +
+        'class MultiSetWidget(widgets.Widget):\n' +
+        '    _view_name = Unicode("MultiSetView", sync=True)\n' +
+        '    a = CInt(0, sync=True)\n' +
+        '    b = CInt(0, sync=True)\n' +
+        '    c = CInt(0, sync=True)\n' +
+        '    d = CInt(-1, sync=True)\n' + // See if it sends a full state.
+        '    def _handle_receive_state(self, sync_data):\n' +
+        '        widgets.Widget._handle_receive_state(self, sync_data)\n'+
+        '        self.d = len(sync_data)\n' +
+        'multiset = MultiSetWidget()\n' +
+        'display(multiset)\n' +
+        'print(multiset.model_id)');
+    this.execute_cell_then(multiset.index, function(index) {
+        multiset.model_id = this.get_output_cell(index).text.trim();
+    });
+
+    this.wait_for_widget(multiset);
+
+    index = this.append_cell(
+        'print("%d%d%d" % (multiset.a, multiset.b, multiset.c))');
+    this.execute_cell_then(index, function(index) {
+        this.test.assertEquals(this.get_output_cell(index).text.trim(), '123',
+            'Multiple model.set calls and one view.touch update state in back-end.');
+    });
+
+    index = this.append_cell(
+        'print("%d" % (multiset.d))');
+    this.execute_cell_then(index, function(index) {
+        this.test.assertEquals(this.get_output_cell(index).text.trim(), '3',
+            'Multiple model.set calls sent a partial state.');
     });
 
     var textbox = {};