Make WordPress Core

Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#17399 closed defect (bug) (fixed)

Esc-key to close modal window in fullscreen mode, closes fullscreen mode.

Reported by: xibe's profile xibe Owned by: azaozz's profile azaozz
Milestone: 3.2 Priority: normal
Severity: normal Version: 3.2
Component: Editor Keywords: has-patch
Focuses: Cc:

Description

I've long been used to close modal windows with the Escape key: Insert/Edit Link, Add Image, Help... all allowed me to close them while keeping my hands on the keyboard.

Now in fullscreen mode, it still works... for the Help window. For the Insert Link and Add Image windows, pressing the Esc-key will both close the modal window AND return the user to fullscreen.

This is in 3.2-beta1, Win7, FFox 4.01.

Attachments (1)

17399-fix.diff (3.2 KB) - added by chrisbliss18 13 years ago.
Patch proposal

Download all attachments as: .zip

Change History (8)

#1 @nacin
13 years ago

  • Milestone changed from Awaiting Review to 3.2

#2 @xibe
13 years ago

Sorry, I meant to say...

AND return the user to normal mode.

@chrisbliss18
13 years ago

Patch proposal

#3 @chrisbliss18
13 years ago

  • Cc gaarai@… added
  • Keywords has-patch added

Submitted the patch proposal 17399-fix.diff. I had to do some things that I'm not sure are acceptable to make it work.

Key modifications:

  • Added a global scope JS variable wpDialogOpen. This is set to false by default in all code files that check it (just to make sure that whatever combination of code is run will have this set by default).
  • The Thickbox and TinyMCE wpDialog APIs were updated to set wpDialogOpen to true when they are opened and false when they are closed. When setting the wpDialogOpen variable to false, a timer of 100 ms is used to ensure that the fullscreen API's event handler can execute before the flag is cleared.
  • The fullscreen API's Esc key event handler was moved from keyup to keydown to keep it inline with how the Thickbox and TinyMCE wpDialog APIs use keydown event handlers. Without this modification, a lengthy timout or complex set of additional event handlers would have to be used to keep the wpDialogOpen flag updates and checks in the correct order.

I considered using some combination of event registrations and handlers to observe when dialogs are open rather than the wpDialogOpen variable, but this would have required even larger modifications and would possibly introduce issues with code requiring APIs to be loaded just to handle this type of setup. This being the case, I opted for simplicity with minimal modifications and requirements.

I'm not attached to the name of the variable, the presence of or duration of the settimeout, or really any of this code.

This patch has been tested in Firefox 4.0.1 (Win 7 & Ubuntu 10.10), Chromium 12.0.742.30 (84361) (Ubuntu 10.10), Chrome 11.0.696.68 (Win 7), Opera 11.10 (Win 7 & Ubuntu 10.10), Safari 5.0.5 (7533.21.1) (Win 7), and IE 9.0.8112.16421 (Win 7). It worked properly in all browsers. IE 9 had some issues in fullscreen, but from my testing, this had nothing to do with my patch as removing my modifications didn't create different results.

#4 @azaozz
13 years ago

  • Owner set to azaozz
  • Resolution set to fixed
  • Status changed from new to closed

In [18069]:

Don't close DFW when closing modal dialogs with Escape key, fixes #17399

#5 follow-up: @nacin
13 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

s/Colse/Close/?

#6 @azaozz
13 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In [18070]:

s/Colse/Close/, props nacin, fixes #17399

#7 in reply to: ↑ 5 @azaozz
13 years ago

Replying to nacin:
Right, it still worked though :-)

Note: See TracTickets for help on using tickets.