PDA

View Full Version : [OPEN] Modal dialogs can be bypassed



kato
19 Jun 2007, 10:44 PM
Not sure if this has been covered already but modal dialogs can be bypassed by simply using the tab key to cycle through all the underlying links, linked images and form elements on the page. For example, if you open a modal dialog on top of a form, you can press tab until you can access the elements behind the modal dialog/mask.

This behavior has been replicated on FF2 and IE7 using 1.1b1 on Windows.

Animal
20 Jun 2007, 12:26 AM
"modal" dialogs are just divs which have an opaque div placed under them that masks underlying elements from mouse events.

Can you think of a workaround?

Perhaps this before show:



this.reEnable = [];
Ext.fly(document.body).select("input select a button").each(function(el) {
if (!el.dom.disabled) {
el.dom.disabled = true;
reEnable.push(el.dom);
}
});


and this after hide:



Ext.each(this.reEnable, function(el) {
el.disabled = false;
});


?

Might be a bit of a performance hit finding and disabling every <input>, <a>, <select> and <button>.

Should <map>/<area> be added to the DomQuery selector?

Perhaps it could be added but made optional?

kato
20 Jun 2007, 1:44 AM
Thanks, Animal. I haven't deeply pondered on a workaround yet, as I was half-hoping that this issue has already been addressed. Your workaround seems feasible and sure worth the try, though it might indeed have some performance degradation on complex layouts/forms in the background, and also complications in nested modal windows. What I was pondering on was to listen for tab key presses and allow focus switching only within the elements of the current modal window (dialog or message box).

brian.moeskau
20 Jun 2007, 9:45 AM
If you wanted to try something like this, you'd want to preserve the original disabled state of each element, rather than simply enabling them all after hide.

J.C. Bize
20 Jun 2007, 1:51 PM
I think you'd also want to run that first chunk of code *after* show (rather than before) so that even if there is a delay in disabling all the elements, the user will not notice it.

kato
20 Jun 2007, 2:00 PM
Yeah, and this might cause some complications in nested modal windows if not done right.

I'm still leaning towards simply listening for tab key presses and limit the focus cycling/switching within the elements of the current modal window.

A real modal window shouldn't be circumvented like this since this can actually break the application. I'm actually surprised that this issue hasn't been addressed or escalated.

brian.moeskau
20 Jun 2007, 2:05 PM
A "real modal window" would also not be a div inside a page :)

kato
20 Jun 2007, 3:04 PM
I was describing how a real modal window "behaves" as it's obvious that Javascript modal windows are simulated. But you know what I mean. Having the modal attribute simply means you intended to simulate a real modal window right? Meaning you basically want to shield the rest of the underlying page from the user. So, why not make it behave as such?

Does this mean that I'm the only one who's concerned that someone might break my application just by simply tabbing through the rest of page and pressing space/enter on the focused buttons? Try it out and you'll see what I mean.

Here's a great example of implementing perfectly simulated modal windows: www.meebo.com (http://www.meebo.com) so I don't see the sense why it can't be done with Ext.

Aside from that, the entire UI feels very snappy and behaves almost like a true desktop app. The only drawback is I think it's developed in Dojo and leaks memory like hell, but the functionality and concept is great.

kato
26 Jun 2007, 8:19 PM
Anybody from the Ext team care to look into this? And hopefully not reply with things like "A real modal window would also not be a div inside a page" as if it were a constructive statement.

brian.moeskau
26 Jun 2007, 8:46 PM
Note the :) denoting that it was a joke...

We can look into it, but it's not the highest priority item on our plate right now. Unless anyone already knows of a super-simple, cross-browser solution that still allows for full keyboard navigation within the dialog itself... ?

brian.moeskau
26 Jun 2007, 8:50 PM
BTW, I looked at Meebo, but I cannot find a modal dialog anywhere, just a bunch of floating windows. Where is the example you mentioned?

Animal
27 Jun 2007, 3:22 AM
If you wanted to try something like this, you'd want to preserve the original disabled state of each element, rather than simply enabling them all after hide.

The code I posted only re-enables those items that were disabled on dialog show.

I'd say that's the best way to do it. It is basically what's needed. All input items under the mask to be disabled.

Another option is a document keydown handler enabled when a modal dialog is shown (perhaps by the DialogManager) which stops any keydown event if the target is not within an element of selector ".x-dlg#" + topMostDialog.id

Where the DialogManager knows which is the topmost Dialog.



if (!Ext.fly(event.getTarget()).findParent(".x-dlg#" + topmostDialog.id, document.body)) {
event.stopEvent();
}

brian.moeskau
27 Jun 2007, 3:35 AM
The code I posted only re-enables those items that were disabled on dialog show.

Right you are. That's what I get for skimming ;)

I was looking at Element.mask -- I think a truly generic solution should block the keyboard for any section of the DOM that is masked, not just beneath a dialog/window. One complication is that different areas within the source do masking in slightly different ways (not everything goes through Element.mask, some components create their own masks manually). The selector filtering approach might work if you can filter for anything beneath the mask el, but I haven't studied the code enough yet to know how that would work.

Tim Siney
27 Jun 2007, 4:46 AM
It struck me a simple solution to this would be to have a listener applied to the dialog div/object which is fired when it looses focus. That way as soon as the user tabbed to an underlying field, the focus would be re-applied to the dialog, making it appear truly modal. The psuedo-code would be thus:


dialog.onBlur(){
dialog.focus();
}

Makes sense to me, but how that would be applied to Ext I don't know. There is some pretty clever Javascript voodoo going on in there, so no doubt it wouldn't be as simple as first appears!

brian.moeskau
27 Jun 2007, 5:10 AM
For a dialog, something like that might work, but there are other situations to handle aside from just dialogs. I still think that the ideal solution would handle disabling any area that is masked, regardless of whether or not there's something modal happening overhead (could just be a masked data loading operation for example). The only thing that you can really rely on being available in every situation is the mask el itself.

kato
27 Jun 2007, 5:46 AM
BTW, I looked at Meebo, but I cannot find a modal dialog anywhere, just a bunch of floating windows. Where is the example you mentioned?

Hi Brian, it's when you try to send a message (double click on a nick/user) - it opens up a modal window where tabbing seems to be restricted only to that window's elements.

here's a simpler example: http://wildbit.com/demos/modalbox/

kato
11 Aug 2007, 2:03 PM
Just wondering if this has been resolved in the latest (unreleased) builds yet...

bearmonger
26 Sep 2007, 8:01 AM
This still doesn't seem to be working in 1.1.1 - are there any updates on progress?

bearmonger
26 Sep 2007, 8:05 AM
It struck me a simple solution to this would be to have a listener applied to the dialog div/object which is fired when it looses focus. That way as soon as the user tabbed to an underlying field, the focus would be re-applied to the dialog, making it appear truly modal. The psuedo-code would be thus:


dialog.onBlur(){
dialog.focus();
}

Makes sense to me, but how that would be applied to Ext I don't know. There is some pretty clever Javascript voodoo going on in there, so no doubt it wouldn't be as simple as first appears!

By the way, Tim's suggestion doesn't seem possible as the Dialog class doesn't expose any events for blur and focus.

lgerndt
28 Nov 2007, 9:27 PM
Our QA department really wants this fixed too, and I tried Animals excellent suggestion of disabling the elements, but I found that for anchor tags at least, even though its "disabled" is true, hitting return while that element is focused still behaves enabled

jdupont092
29 Nov 2007, 12:26 AM
I had to face this exact issue in my own web application (I'm not using extjs).
I was originally using the same hack to simulate modal dialogs (ie, putting a 50% transparency div over the whole page) then discovered that TAB could lead to serious problems (for exemple, i used modals to allow users to change property on selected elements, but they could change the actual selected element using the TAB key AFTER the modal appears).

So i decided to make it less appealing but more stable : i now hide the whole page (mainPanel.style.visiblity='hidden') then show it again on modal close (mainPanel.style.visibility='').
Works like a charm, even if it's less eye candy.

jdupont092
29 Nov 2007, 1:15 AM
Some adds :
Modal Windows does NOT disable every window of an application : they only disable their parent window.
You can have two normal windows shown in your application, then one of them would pop a modal, then only this window would be disabled, not the other.
At the same time, if the other window also pop a modal, and then you close only one of those modal, only the parent of the modal closed is re-enabled, not every window.
Quick Example :
Open firefox, go to a website. Open the Download window using Tools/Downloads. This window is not a modal one, you can still use your main Ffx window. Then right click on a downloaded element inside the download window, and choose 'Properties'. The property window IS a modal, but only the download window is unreachable, the Ffx window is still usable.

This means that you have to track your parent window for every modal :

Inside the main window (yeah, even the main application should at least implements DisableWindow/EnableWindow) :
function DoSomething()
{
.....
ShowModal(newWindow,this);
.....
}

//Static ShowModal function with :
//win : window to show in modal mode.
//parent : parent of the modal window.
function ShowModal(win, parent)
{
parent.DisableWindow();
win.IsModal=true;
win.Parent=parent;//So that i can find which window i'll have to re-enable on close...
win.Show(); //Function of the window object
}

window.prototype.DisableWindow=function()
{
//As stated before, i use the hiding way
this.mainDivElement.style.visibility='hidden';
}

window.prototype.EnableWindow=function()
{
this.mainDivElement.style.visibility='';
}


window.prototype.Close=function ()
{
if (this.IsModal)
{
//Re-enable my parent.
this.Parent.EnableWindow();
}
//Standard close processing
.....
}

window.prototype.Show=function()
{
//Standard show processing
...
}


Just a thought.

jdupont092
29 Nov 2007, 1:59 AM
DP

slobo
29 Nov 2007, 8:28 AM
I have the same problem
modal windows not being really modal

just a trivial example, with a MessagBox:
if I have two windows
and I show a MessageBox
the MessageBox goes BEHIND one of the windows
so it is not visible to the user...

[2.0rc1]

mystix
29 Nov 2007, 8:58 AM
I have the same problem
modal windows not being really modal

just a trivial example, with a MessagBox:
if I have two windows
and I show a MessageBox
the MessageBox goes BEHIND one of the windows
so it is not visible to the user...

[2.0rc1]

you might want to post in the 2.0 Bugs forum instead with a link back to this thread.

slobo
29 Nov 2007, 11:30 AM
my previous entry was a fault alarm
sorry

in the desktop sample, the x-desktop CSS class NEEDS the
position: relative;
style (it has), otherwise the z-index stack gets confused...

but the taskbar and the start menu is still accessible
in case of a modal window...

slobo
6 Dec 2007, 5:59 AM
modal windows can still be bypassed with the TAB key in 2.0 final

kato
10 Jan 2008, 11:11 AM
any news? i too was hoping that this would be taken with a bit more consideration in 2.0...

dancas
6 Feb 2008, 11:57 AM
This bug was not solved yet?

thejoker101
6 Feb 2008, 1:12 PM
I have the same problem
modal windows not being really modal

just a trivial example, with a MessagBox:
if I have two windows
and I show a MessageBox
the MessageBox goes BEHIND one of the windows
so it is not visible to the user...

[2.0rc1]
Not sure if you figured this out, but I had posted a thread about this a couple days ago and Jack replied yesterday saying to set the zseed on your WindowGroup to something lower. Or I just learned there's a default WindowMgr (hadn't noticed it and I think all the desktop examples use their own WindowGroup).

FxMan
12 Mar 2008, 6:44 AM
http://extjs.com/forum/showthread.php?t=29231

aaaee2
24 Jun 2008, 6:14 AM
but the taskbar and the start menu is still accessible
in case of a modal window...Sorry, I'm very newbie in development of Ext applications. Next code (diff) is my little dirty hack that I use in my application based on desktop example:
Index: taskbar.js
===================================================================
--- taskbar.js (revision 1)
+++ taskbar.js (working copy)
@@ -383,6 +383,7 @@
* @class Ext.ux.TaskBar.TaskButton
* @extends Ext.Button
*/
+var _modalWinTitleSpanEl;
Ext.ux.TaskBar.TaskButton = function(win, el){
this.win = win;
Ext.ux.TaskBar.TaskButton.superclass.constructor.call(this, {
@@ -390,12 +391,23 @@
text: Ext.util.Format.ellipsis(win.title, 12),
renderTo: el,
handler : function(){
- if(win.minimized || win.hidden){
- win.show();
- }else if(win == win.manager.getActive()){
- win.minimize();
- }else{
- win.toFront();
+ var activeWindow = win.manager.getActive();
+ if (activeWindow && !activeWindow.modal) {
+ if(win.minimized || win.hidden){
+ win.show();
+ }else if(win == win.manager.getActive()){
+ win.minimize();
+ }else{
+ win.toFront();
+ }
+ } else {
+ var _t = Ext.query('div#' + activeWindow.id + ' div.x-window-tl div.x-window-tr div.x-window-tc div:only-child span:last-child');
+ _modalWinTitleSpanEl = Ext.get(_t[0]);
+ for (var i=200;i<=1200;i+=200) {
+ setTimeout("_modalWinTitleSpanEl.replaceClass('x-window-header-text', 'x-window-header-text-masked')", i);
+ i+=200;
+ setTimeout("_modalWinTitleSpanEl.replaceClass('x-window-header-text-masked', 'x-window-header-text')", i);
+ }
}
},
clickEvent:'mousedown',
This code forbids to be switched to other windows by a click in taskbar and does the text of heading of a modal window blinking. Also you need to add class x-window-header-text-masked, which contains "color:gray;".

Is there any nice solution for this task?

Ian Grainger
11 Mar 2010, 7:13 AM
Posted my fix based on Animal's approach in #2: here (http://www.extjs.com/forum/showthread.php?p=445782#post445782).

Ian Grainger
11 Mar 2010, 7:14 AM
Posted my solution to this: here (http://www.extjs.com/forum/showthread.php?p=445782#post445782).