Update Backbone.js to 0.9.10

[markjaquith]

Backbone.js 0.9.9 and 0.9.10 have some cool stuff. We should stay up to date.

[aaronholbrook]

23262.patch

[aaronholbrook]

Apologies for the double upload, first time manually uploading a diff.

[aaronholbrook]

Please ignore the backbone-min.js file, after talking with @evan, realized keeping the WP name was the smart thing to do.

[aaronholbrook]

Fixed array styling (spaces between parens) at suggestion of Mr. Solomon.

Thanks Evan :)

[nacin]

In 23375:

Update Backbone.js to 0.9.10. props aaronholbrook, fixes #23262.

[markoheijnen]

Media doesn't work for me on several trunk sites. Reverting this change does work for me.

[scribu]

Works fine for me in both Firefox and Chrome. Could you describe how exactly it "doesn't work"? Any JS errors?

[scribu]

Nevermind, I was still running the old Backbone version. When I press the "Add Media" button, I get a JS error:

TypeError: e is undefined

from underscore.min.js.

[markoheijnen]

Add media button:

Uncaught TypeError: Cannot read property 'map' of undefined underscore.min.js:5
T.map.T.collect underscore.min.js:5
T.(anonymous function) underscore.min.js:5
media.model.Attachments.Backbone.Collection.extend._changeFilteredProps media-models.js:395
w backbone.min.js:8
g.Events.trigger backbone.min.js:10
f.extend.set backbone.min.js:14
media.model.Attachments.Backbone.Collection.extend.initialize media-models.js:353
media.model.Query.Attachments.extend.initialize media-models.js:661
g.Collection backbone.min.js:18
d backbone.min.js:41
d backbone.min.js:41
media.model.Query.Attachments.extend.get media-models.js:833
media.model.Attachments.Backbone.Collection.extend._requery media-models.js:556
media.model.Attachments.Backbone.Collection.extend._changeQuery media-models.js:383
w backbone.min.js:8
g.Events.trigger backbone.min.js:10
f.extend.set backbone.min.js:13
media.model.Attachments.Backbone.Collection.extend.initialize media-models.js:353
g.Collection backbone.min.js:18
d backbone.min.js:41
media.query media-models.js:642
media.view.MediaFrame.Post.media.view.MediaFrame.Select.extend.createStates media-views.js:1611
media.view.MediaFrame.Select.media.view.MediaFrame.extend.initialize media-views.js:1486
media.view.MediaFrame.Post.media.view.MediaFrame.Select.extend.initialize media-views.js:1595
g.View backbone.min.js:36
media.View.Backbone.View.extend.constructor media-views.js:1190
d backbone.min.js:41
d backbone.min.js:41
d backbone.min.js:41
d backbone.min.js:41
wp.media media-models.js:30
wp.media.editor.add media-editor.js:455
wp.media.editor.open media-editor.js:622
(anonymous function) media-editor.js:641
v.event.dispatch jquery.js:2
o.handle.u jquery.js:2

Featured image

Uncaught TypeError: Cannot call method 'on' of undefined media-views.js:319
media.controller.State.Backbone.Model.extend._updateMenu media-views.js:319
w backbone.min.js:8
g.Events.trigger backbone.min.js:10
f.extend.set backbone.min.js:13
g.Model backbone.min.js:12
media.controller.State.Backbone.Model.extend.constructor media-views.js:229
d backbone.min.js:41
d backbone.min.js:41
wp.media.featuredImage.frame media-editor.js:351
$.on.on.wp.media.view.settings.post.featuredImageId media-editor.js:382
v.event.dispatch jquery.js:2
o.handle.u jquery.js:2

[scribu]

Updating to underscore 1.4.4 makes the featured image link work again. see underscore-1.4.4.diff​.

However, Add Media still doesn't work and no JS error now, which sucks.

[markjaquith]

Your patch had malformed JS, scribu. Still have issues, but at least underscore parses.

[markjaquith]

This is the Backbone commit that broke it. The issue appears to be unrelated to Underscore.

​https://github.com/documentcloud/backbone/commit/71b6404d73f506627f0cc516177e39cc8c9947d7

[markjaquith]

In 23383:

Update Underscore to version 1.4.4. see #23262

[scribu]

Getting a different error after [23383], in Firefox:

TypeError: this.frame is undefined
this.frame.on( 'menu:render:' + menu, this._renderMenu, this );

[Ipstenu]

Still broke on Chrome too. I'm about to fly out the door, but I svn'd up and it's still not opening the modal.

[davouch]

#23386 was marked as a duplicate.

[azaozz]

There are quite a few changes in ​Backbone 0.9.10:

View#make has been removed. You'll need to use $ directly to construct DOM elements now.

We use that in few places.

The Model#change method has been removed, as delayed attribute changes as no longer available.

And that.

Also it seems change event on collections now fires on creating the collection, before we can add this.frame throwing the above error.

[ocean90]

Related: ​http://addyosmani.github.com/backbone-fundamentals/#upgrading-to-backbone-0.9.10

[scribu]

I think we should revert [23383] and [23375], so that trunk doesn't stay broken until we come up with a properly tested patch.

[azaozz]

Think so too. Had a better look and even after patching the obvious places, there are still errors.

[redsweater]

I looked into this briefly. The firing of the change event as alluded to by azaozz above does indeed cause the this._updateMenu callback to be reached earlier than it was previously.

I don't know enough about the code, or about Backbone to make very informed choices about reorganizing this, but I did notice for example if I move the line:

this.on( 'change:menu', this._updateMenu, this );

Out of the media controller's constructor, and into _ready():

_ready: function() {
	this._updateMenu();
	this.on( 'change:menu', this._updateMenu, this );
},

Then at least the UI is allowed to complete instantiation without exceptions, and the media window appears. Of course, as predicted, there are further issues within once you try to navigate around the media window.

Seems like more than one nuanced bad behavior may boil down to assumptions in different controllers that change events will come after the add events. In this particular case, doesn't it make sense to at least push off the registration of the this._updateMenu callback until after the menu has been initialized in _ready()?

[Ipstenu]

Can we revert this to unbreak trunk?

[markjaquith]

In 23390:

Revert [23375] until we can get Media fixed to work with newer versions of Backbone. see #23262

[scribu]

It seems the underscore.js upgrade didn't have any adverse effects, so we don't need to revert that.

[gcorne]

I spent some time looking at this today. In addition to the Backbone changes that @azaozz mentioned above, there are also some changes to Backbone.sync that impact the overridden .sync() methods and .getByCid() has been removed in favor of just using .get()

I put together a patch that seems to be working, although I haven't tested it extensively. My work can also be seen in a branch on GitHub ​https://github.com/gcorne/WordPress/commits/backbone-upgrade for those that prefer a bit more detail.

[koopersmith]

In 23589:

Update Backbone to 0.9.10. Update media to handle breaking changes.

• When overriding Backbone.sync(), ensure the sync and error events fire consistently.

• Model#make() has been removed. Use $ instead, and be sure to grab the DOM node where necessary (using [0]).

• Collection#get() now accepts cids. Collection#getByCid() has been removed.

• When overriding the State constructor, bind change callbacks after the default Model constructor is called, because the Model constructor no longer passes the silent flag when calling set() for the default attributes.

• In 'change' events, options.changes was removed. It can now be accessed through model.changed. Check if any attributes have changed by calling model.hasChanged(). Also, don't mess with model.changed; it persists beyond the scope of a single event.

• options.index is no longer be set in the add event callback. Use collection.indexOf(model) can be used to retrieve the index of a model instead.

props gcorne. fixes #23262.