Customizer settings with non-scalar values incorrectly trigger as changed

[westonruter]

When a value is updated in the customizer models, it checks to see if the new value equals the old value, and if it does, then it short-circuits so as to not trigger the change callbacks if the new value is not different. This is working fine for scalar values (e.g. strings, booleans, numbers). However, for objects the test fails because two JavaScript object instances with the same contents are still different instances, and so they are not identical.

The current equality test in customize.Value is:

if ( null === to || this._value === to )

But if if the old value was an object {"title":"Foo"} and the new value is {"title":"Foo"}, then this conditional will fail because in JavaScript: {"title":"Foo"} !== {"title":"Foo"}. The result is that the value change callbacks will fire even the value didn't change, including when the customizer preview first loads as the settings get synced over.

Fortunately, Underscore.js provides an isEqual function to compare two values to see if they have the same contents, and so we can use this instead:

if ( null === to || _.isEqual( from, to ) ) {

Introduced in #19910 via r19995

This bug was discovered in the course of developing the Widget Customizer plugin, as each widget instance is stored in a customizer setting, and these instances are JavaScript objects.

[ocean90]

@westonruter Could you please provide a simple demo plugin which currently fails?

I'm not sure if we should enqueue Underscore.js for just one method.

[westonruter]

@ocean90: here is a quick and dirty demo plugin: ​https://gist.github.com/westonruter/7729203

You'll notice that if you hit the Update button in the control, that that the preview reloads and the customizer's state changes to unsaved, even though none of the setting's properties were changed.

As for not enqueueing all of Underscore.js for this one function <​http://underscorejs.org/docs/underscore.html#section-84>, we could copy it out and include it in customize-base.js, maybe as api.Value.isEqual( to )

[ocean90]

Thanks westonruter.

26061.2.patch​ includes the copy of eq from Underscore.js. Seems to work.

[westonruter]

@ocean90: Looks perfect!

[ocean90]

In 26548:

Customizer: Don't trigger a change event if two unchanged object values are equal.

In JavaScript: {"title":"Foo"} !== {"title":"Foo"}
Underscore.js provides an isEqual function to compare two values to see if they have the same contents, which can be use instead.
Because only one function of Underscore.js is needed we just add a copy of _.isEqual to customize-base.js.

props westonruter.
fixes #26061.

[JustinSainton]

I'm more than happy to be the only one who thinks this...but...

Doesn't it seem kind of lame to copy massive chunks of Underscores.js? Would it be so bad to just have it globally enqueued, like jQuery (except jQuery isn't enqueued on the front-end, but you get my point)?

Like I said, happy to be alone on this, but I think having Underscores.js available everywhere would actually be a pretty huge win.

[atimmer]

[26548] introduced a jshint error, ​26061.jshint.patch fixes it.

[SergeyBiryukov]

In 26632:

Fix JSHint error. props atimmer. see #26061.

[nacin]

In 26702:

Customizer: Revert [26548], removing _.isEqual() for proper object comparison.

This broke the customizer in IE 11, with possibly other side effects. Revisit in 3.9.

Also reverts [26632].

see #26061 (reopens), fixes #26438.

[westonruter]

@nacin Do we have details on what broke specifically in IE 11? If isEqual broke the Customizer, then perhaps the other instances of _.isEqual() are breaking IE 11 too. For instance:

src/wp-includes/js/media-models.js
65:		if ( _.isEqual( a, b ) )
502:				if ( ! _.isEqual( attachment.attributes, newAttributes ) )
783:					return _.isEqual( query.args, args );

src/wp-includes/js/shortcode.js
219:		} else if ( _.isEqual( _.keys( attrs ), [ 'named', 'numeric' ] ) ) {

[nacin]

ocean90 tested media and it appeared to work OK. Possibly because of the data types we were using with it. There's more of an explanation for what broke in #26438.

[SergeyBiryukov]

According to the comments on #26438, updating Underscore.js to 1.5.2 would fix this.

[ocean90]

According to the comments on #26438, updating Underscore.js to 1.5.2 would fix this.

Not sure, since the included part was from 1.5.2.

[SergeyBiryukov]

Replying to ocean90:

Not sure, since the included part was from 1.5.2.

I see, I misread the comment in ticket:26438:26438.patch​. Thanks for the clarification.

[markjaquith]

Need more information about the IE 11 issue.

[celloexpressions]

I believe we're nearly always enqueuing Underscore.js in the Customizer now for Widgets (along with Backbone.js), so we might as well use that version of isEqual here (#29572 also requires Underscore.js, via wp-util.js, which is being enqueued already anyway).

26061.3.diff​ ended up just being a refresh of westonruter's original patch, and doesn't break IE11 for whatever reason. If we get this in soon someone'll definitely run into any bugs, but I'm guessing this should work fine now.

[ocean90]

In 29907:

Customizer: Don't trigger a change event if two unchanged object values are equal, second pass.

Make Underscore.js a dependency for customize-base and use _.isEqual() to compare the values.
(Underscore.js was already enqueued via wp-util.js for Widgets.)

props westonruter.
fixes #26061.