Opened 12 years ago

Closed 10 years ago

#6184 closed defect (fixed)

[patch] [no cla] Dialog: ESC doesn't always close popup dialogs in Safari

Reported by: Douglas Hays Owned by: Douglas Hays
Priority: high Milestone: 1.4
Component: Dijit Version: 1.0
Keywords: Cc: Becky Gibson
Blocked By: Blocking:

Description (last modified by bill)

Confirmed using Safari 3.0.4
1) Open test_Dialog.html
2) Click Show Dialog
3) Clicking whitespace in dialog (so that focus is not on a field)
4) press ESC
Expected: Dialog should close
Actual: nothing.

Attachments (2)

FocusOnBodyEl.html (866 bytes) - added by Joseph Scheuhammer 11 years ago.
Tests programmatically moving focus to the <body> element.
dojo.escape.patch (730 bytes) - added by nqzero 10 years ago.
make google-chrome (and safari ?) honor escape in popups.js

Download all attachments as: .zip

Change History (21)

comment:1 Changed 12 years ago by Douglas Hays

Description: modified (diff)

comment:2 Changed 11 years ago by bill

Component: GeneralDijit
Description: modified (diff)
Summary: ESC doesn't always close popup dialogs in SafariDialog: ESC doesn't always close popup dialogs in Safari

comment:3 Changed 11 years ago by bill

The problem w/safari, compared to other browsers, is that clicking on a blank area of the Dialog doesn't focus the dialog itself. Rather, focus is completely lost. (You can see this by hitting tab; focus goes to the next fields behind the Dialog.) So I assume this is just a manifestation of a safari issue where you can't focus non-input elements?

Note also that for any browser, if you click a blank area of the screen not on the Dialog, then ESC doesn't work. Maybe the ESC handler should be on <body> rather than on the Dialog.domNode?

comment:4 Changed 11 years ago by bill

Milestone: 1.21.3
Owner: davidb deleted

Hmm, it turns out that the ESC handler is on <body>, in addition to the Dialog domNode itself. (That should be fixed too; there's no need to connect twice, is there?) Maybe can attach a handler to the underlay instead of <body>?

Anyway, unassigning this from David since he isn't working on Dojo stuff right now, and bumping to 1.3.

comment:5 Changed 11 years ago by Joseph Scheuhammer

Regarding Safari: having an ESC handler on <body> in Safari won't work since Safari allows focus for only form (e.g., <input>) and <a> elements. In particular <body> cannot receive focus. See the attached test file, tested on Safari 3.1.2, Mac OS 10.5.4.

This might fall under the general need for Safari/Webkit? to support ARIA, which would include allowing focus on any element: https://bugs.webkit.org/show_bug.cgi?id=12132

Changed 11 years ago by Joseph Scheuhammer

Attachment: FocusOnBodyEl.html added

Tests programmatically moving focus to the <body> element.

comment:6 Changed 11 years ago by bill

Milestone: 1.31.5

comment:7 Changed 11 years ago by Douglas Hays

Cc: Becky Gibson added

I had an idea for a webkit hack to workaround this:

<input id="_webkit_hack_" readonly tabIndex=-1 
style="position:absolute;outline:0px none;width:1px;height:1px;padding:0px;margin:0px;border:0px none;background-color:transparent;">

You can't tab to this input put you can call focus() on it and intercept keys once focused - and there's no blue/yellow focus border. So it'll act like having the document body focused in other browsers.

comment:8 in reply to:  7 Changed 11 years ago by Joseph Scheuhammer

Replying to doughays:

I had an idea for a webkit hack to workaround this: ...

Interesting. A couple of comments.

First, given that Safari 4 does support focus on all elements, will this be necessary? It depends on when dojo v1.5 is slated for release. (The FocusOnBodyEl.html example sort of works with Safari 4beta).

Second, why not style="display: none;"? Would that suffice?

comment:9 Changed 11 years ago by Douglas Hays

I ran test_Dialog.html with Safari 4 and ESC still does not close the dialogs, so I think this solution may be needed there afterall. display:none doesn't work since you cannot focus and type (ESC) in a display:none textbox.

comment:10 in reply to:  9 Changed 11 years ago by Joseph Scheuhammer

Replying to doughays:

I ran test_Dialog.html with Safari 4 and ESC still does not close the dialogs ...

ESC closed most of the dialogs for me (Safari 4 beta Mac OSX 10.5.6). The one that had a problem was the one invoked from the "Programmatic Dialog (3 second delay)" button. For that one, if the focus is on the input text field, ESC is ignored. If focus is moved to away from the text, say the "hello" button below, then ESC closes that dialog as well.

However, it's not as consistent on WinXP. ESC closed only the dialogs invoked from the "Show Dialog" and "Show File Dialog" buttons.

A question is, will this be fixed in Webkit for the Safari 4 release? I'll see what I can find out.

comment:11 Changed 11 years ago by Becky Gibson

Note that the behavior for dialog boxes with no focusable element should be changing in 1.4 - #8285. That may affect this discussion and fix.

comment:12 Changed 10 years ago by roman2

I see that when a dialog's field has focus, pressing Escape key still doesn't close the dialog.

comment:13 Changed 10 years ago by nqzero

just attached the google-chrome patch. i didn't know how to capability-sniff, so i just browser sniffed. don't think it will cause any trouble if/when google fixes chrome. i don't have safari to test with, but i'm guessing that it works

Changed 10 years ago by nqzero

Attachment: dojo.escape.patch added

make google-chrome (and safari ?) honor escape in popups.js

comment:14 Changed 10 years ago by bill

Summary: Dialog: ESC doesn't always close popup dialogs in Safari[patch] [no cla] Dialog: ESC doesn't always close popup dialogs in Safari

Hello Seth Lytle,

Thanks for working on this! I wonder if this code should be in dojo/_base (either connect.js or event.js, I forget), since it's a general problem with ESC not limited to closing popups. There's various code there for normalizing keydown/keypress.

Also, apparently you http://dojotoolkit.org/dojo-contributors don't have a CLA, you should http://dojofoundation.org/cla/ file one before submitting patches.

BTW, dojo.isWebkit encompasses both Safari and Chrome.

comment:15 Changed 10 years ago by bill

Note that Doug recently added keyDown code in popup.js for webkit, in [20098]:

if(dojo.isWebKit && args.onCancel){ // ESC is only stoppable on keydown and not keypress on WebKit (ignored completely on older Safari)
	handlers.push(dojo.connect(wrapper, "onkeydown", this, function(evt){
		if(evt.keyCode == dojo.keys.ESCAPE){
			dojo.stopEvent(evt);
			args.onCancel();
		}
	}));
}

Also note, the root ESC problem is filed against core in #9528.

comment:16 Changed 10 years ago by Douglas Hays

Milestone: 1.51.4
Owner: set to Douglas Hays
Status: newassigned

comment:17 Changed 10 years ago by Douglas Hays

Resolution: fixed
Status: assignedclosed

(In [20306]) Fixes #6184, #9528, #6184 !strict. Forced Esc key to generate fake keypress on WebKit?. Removed keydown special WebKit/Esc? processing from dijit's popup.js and _FormWidget.js. Verified that dijit's robot tests ComboBox_a11y.html, Dialog_a11y.html, and Menu_a11y.html now run without errors on Safari4/Windows.

comment:18 Changed 10 years ago by dante

Resolution: fixed
Status: closedreopened

this fixes Chrome/Chromium?, but I'm still seeing the original report in Safari4 (went testing Lightbox on a related issue).

Saf4 -> test_Dialog.html -> "Show TabContainer? Dialog" -> ESC ... does not close.

comment:19 Changed 10 years ago by dante

Resolution: fixed
Status: reopenedclosed

I apologize. I had not updated to use event.js from trunk apparently. my last report is invalid. Lightbox is also fixed, fwiw. Thanks doug!

Note: See TracTickets for help on using tickets.