Don't close fullscreen mode on Esc when the media modal is shown

[kovshenin]

This has been reported and fixed previously for Thickbox in #17399, but with the new media modal, it's a little bit more complicated.

Steps to reproduce:

1. Create a new post
2. Select Text mode and enable fullscreen mode
3. Hit the Add Media button
4. Hit the Escape key on your keyboard

Both the media modal and fullscreen mode will close. Only the media modal is expected to close.

[kovshenin]

In 26854.diff​: stop event propagation on media modal close with the Escape key. Also use the keydown event to close fullscreen instead of keyup, otherwise it's a different event.

[nacin]

26854.diff​ looks good. (That could actually be converted to return false which implies stopPropagation and preventDefault.) What's the purpose of the changes in wp-fullscreen.js?

[nacin]

Discussed in IRC, I got it now. keydown versus keyup. Patch looks good.

It would be nice if our handling of Esc are sane across .wp-dialog, fullscreen, the media modal, thickbox (and customizer?). This patch incidentally moves around code that accounts for two of those.

[kovshenin]

Updated in 26854.2.diff​ to address thickbox and wplink, which also removes the CloseOnEscape hack in common.js introduced in r18069. Tested in latest Chrome, Firefox and Safari on OS X, didn't test in IE.

[azaozz]

Been thinking about this: the problem is there's no "priority" on the attached events. Doing stopImmediatePropagation() or returning false only works for the handlers attached after the current one. A handler attached before the current one fires anyways.

The only way to manage the firing order is by controlling when the script is outputted or attaching some to keyup, some to keydown, some to the body and some to the document.

[azaozz]

In 27080:

Consolidate "close on Escape" in the media modal, DFW, wpLink and Thickbox. Props kovshenin, fixes #26854.