Opened 4 months ago
Closed 3 months ago
#23262 closed task (blessed) (fixed)
Update Backbone.js to 0.9.10
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Priority: | normal | Milestone: | 3.6 |
| Component: | External Libraries | Version: | 3.5 |
| Severity: | normal | Keywords: | needs-patch |
| Cc: | david@…, kadamwhite, gregory@… |
Description
Backbone.js 0.9.9 and 0.9.10 have some cool stuff. We should stay up to date.
Attachments (8)
Change History (38)
aaronholbrook — 4 months ago
aaronholbrook — 4 months ago
aaronholbrook — 4 months ago
comment:1
aaronholbrook — 4 months ago
- Keywords has-patch added; needs-patch removed
Apologies for the double upload, first time manually uploading a diff.
comment:2
aaronholbrook — 4 months ago
Please ignore the backbone-min.js file, after talking with @evan, realized keeping the WP name was the smart thing to do.
aaronholbrook — 4 months ago
comment:3
aaronholbrook — 4 months ago
Fixed array styling (spaces between parens) at suggestion of Mr. Solomon.
Thanks Evan :)
aaronholbrook — 4 months ago
- Owner set to nacin
- Resolution set to fixed
- Status changed from new to closed
In 23375:
comment:6
markoheijnen — 4 months ago
- Keywords has-patch commit removed
- Resolution fixed deleted
- Status changed from closed to reopened
Media doesn't work for me on several trunk sites. Reverting this change does work for me.
Works fine for me in both Firefox and Chrome. Could you describe how exactly it "doesn't work"? Any JS errors?
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.
comment:9
markoheijnen — 4 months ago
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
comment:10
scribu — 4 months ago
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 — 4 months ago
comment:11
markjaquith — 4 months ago
Your patch had malformed JS, scribu. Still have issues, but at least underscore parses.
comment:12
markjaquith — 4 months ago
This is the Backbone commit that broke it. The issue appears to be unrelated to Underscore.
https://github.com/documentcloud/backbone/commit/71b6404d73f506627f0cc516177e39cc8c9947d7
comment:13
markjaquith — 4 months ago
In 23383:
comment:14
scribu — 4 months ago
Getting a different error after [23383], in Firefox:
TypeError: this.frame is undefined this.frame.on( 'menu:render:' + menu, this._renderMenu, this );
comment:15
Ipstenu — 3 months ago
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.
comment:16
davouch — 3 months ago
#23386 was marked as a duplicate.
comment:17
scribu — 3 months ago
- Severity changed from normal to blocker
comment:18
davouch — 3 months ago
- Cc david@… added
comment:19
azaozz — 3 months ago
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.
comment:20
ocean90 — 3 months ago
comment:21
scribu — 3 months ago
comment:22
azaozz — 3 months ago
Think so too. Had a better look and even after patching the obvious places, there are still errors.
comment:23
redsweater — 3 months ago
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()?
comment:24
Ipstenu — 3 months ago
Can we revert this to unbreak trunk?
comment:25
markjaquith — 3 months ago
In 23390:
comment:26
scribu — 3 months ago
- Keywords needs-patch added
- Severity changed from blocker to normal
It seems the underscore.js upgrade didn't have any adverse effects, so we don't need to revert that.
comment:27
kadamwhite — 3 months ago
- Cc kadamwhite added
comment:28
gcorne — 3 months ago
- Cc gregory@… added
comment:29
gcorne — 3 months ago
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.
comment:30
koopersmith — 3 months ago
- Resolution set to fixed
- Status changed from reopened to closed
In 23589:

23262.patch