Media grid AttachmentsBrowser arrows navigation and restoreFocus() no longer work

[afercia]

In WordPress 4.4, the media grid allows to navigate attachments in all directions using the Up/Down/Left/Right arrow keys. Also, when in the details sidebar and tabbing backwards, focus is restored to the currently selected attachment.

In WordPress 4.5 these two functionalities no longer work. Sorry can't help more than this, just tracked down to:
in AttachmentsBrowser.createAttachments() which runs on AttachmentsBrowser.initialize()
these two callbacks no longer work:

this.attachments.listenTo( this.controller, 'attachment:keydown:arrow',     this.attachments.arrowEvent );
this.attachments.listenTo( this.controller, 'attachment:details:shift-tab', this.attachments.restoreFocus );

probably related to the changes in Backbone.js 1.2.0, see [36546]

/cc @wonderboymusic @adamsilverstein

[afercia]

Worth considering there are probably other places where this doesn't work any longer.

[adamsilverstein]

@afercia Thanks for the bug report and tracking down the source!

We have seen this type of issue elsewhere in core since upgrading Backbone and tried to track down every case, unfortunately we missed a few. Backbone limited the way events are registered over the two years Core hadn't upgraded (mentioned on the commit you referenced), however this instance doesn't match those patterns.

I will dig in to see how we can fix this. (on a related note, Backbone is now at version 1.3.3 and includes some bug fixes from our bundled version. I will open a ticket to upgrade again for 4.6)

[afercia]

@adamsilverstein thanks :) some more info: I've tried with Backbone 1.3.3 (just overwriting files on my local install) and that didn't help. Tried also to revert Backbone to the version used in 4.4 and everything worked again.

[adamsilverstein]

Thanks, yea confirmed broke in [36546]

[adamsilverstein]

listenTo isn't really required here and seems to be the point of failure, switching to the .on form (and binding the attachments for context) fixes the issue.

In 36900.diff​

• Switch from listenTo to on when binding listeners to handle keyboard navigation and image re-focus in the media grid; the existing listeners broke when we upgraded Backbone in [36546].

Marking for consideration for 4.5.3 since this is a 4.5 regression. @afercia can you please test my patch to verify this fixes the issue you raised?

[afercia]

Works for me. :)
Tested on latest Chrome, Firefox and IE 11. Tested also on IE 8... the whole media modal is so slow that actually it seems freezed... but it works. That's a separate issue but worth considering starting a discussion about browsers support.

[joemcgill]

Patches source file and includes changes to built file.

[joemcgill]

36900.2.diff​ Applies the same changes from 36900.diff​ to the source file, src/wp-includes/js/media/views/attachments/browser.js and I've included the modified file after running grunt precommit so it's easier to apply and test without running a build step.

[adamsilverstein]

Looks good @joemcgill - sorry I am always a little confused how I should provide patches for these compiled media files. I'll try to patch both source and generated in the future to make your job as a committer easier.

[joemcgill]

In 37755:

Media: Restore keyboard navigation of the media grid.

This changes the binding of event listeners in the Attachments Browser
to use on instead of listenTo for the attachment:keydown:arrow and
attachment:details:shift-tab events. The existing listeners broke
when we upgraded Backbone in [36546].

Props adamsilverstein.
Fixes #36900.

[joemcgill]

Still needs commit for 4.5.3.

[swissspidy]

In 37804:

Media: Restore keyboard navigation of the media grid.

This changes the binding of event listeners in the Attachments Browser
to use on instead of listenTo for the attachment:keydown:arrow and
attachment:details:shift-tab events. The existing listeners broke
when we upgraded Backbone in [36546].

Merge of [37755] to the 4.5 branch.

Props adamsilverstein.
Fixes #36900.