Opened 10 years ago

Closed 10 years ago

#10396 closed defect (fixed)

TooltipDialog: crash in IE when open dialog using space key and close via escape key

Reported by: Becky Gibson Owned by: bill
Priority: high Milestone: 1.4
Component: Dijit Version: 1.4.0b
Keywords: Cc: Jared Jurkiewicz
Blocked By: Blocking:

Description

  1. open dijit/tests/test_TooltipDialog.html
  1. tab to "Show Tooltip Dialog" button
  1. press space to open dialog
  1. press escape to close dialog
  1. IE7 crashes immediately, IE8 will crash if you repeat steps 3 and 4 a few times

This may be related to the inline edit box that was added as the first element in the dialog. However, IE 8 also crashes if you to the same sequence on the next dialog with the tab container.

Change History (5)

comment:1 Changed 10 years ago by Jared Jurkiewicz

Owner: set to bill

I think the problem is related to dijit.focus(something) being called inside a key event handler. I was warned a long time ago to not do that, as IE and other browsers do not like changing focus inside an event handler.

To try and test this theory, I modified dijit.focus() thus:

	focus: function(/*Object || DomNode */ handle){
		// summary:
		//		Sets the focused node and the selection according to argument.
		//		To set focus to an iframe's content, pass in the iframe itself.
		// handle:
		//		object returned by get(), or a DomNode
		if(!handle){ return; }

		var node = "node" in handle ? handle.node : handle,		// because handle is either DomNode or a composite object
			bookmark = handle.bookmark,
			openedForWindow = handle.openedForWindow,
			collapsed = bookmark ? bookmark.isCollapsed : false;

		// Set the focus
		// Note that for iframe's we need to use the <iframe> to follow the parentNode chain,
		// but we need to set focus to iframe.contentWindow

		setTimeout(function(){
			if(node){
				var focusNode = (node.tagName.toLowerCase() == "iframe") ? node.contentWindow : node;
				if(focusNode && focusNode.focus){
					try{
						// Gecko throws sometimes if setting focus is impossible,
						// node not displayed or something like that
						focusNode.focus();
					}catch(e){/*quiet*/}
				}
				dijit._onFocusNode(node);
			}

			// set the selection
			// do not need to restore if current selection is not empty
			// (use keyboard to select a menu item) or if previous selection was collapsed
			// as it may cause focus shift (Esp in IE).
			if(bookmark && dojo.withGlobal(openedForWindow || dojo.global, dijit.isCollapsed) && !collapsed){
				if(openedForWindow){
					openedForWindow.focus();
				}
				try{
					dojo.withGlobal(openedForWindow || dojo.global, dijit.moveToBookmark, null, [bookmark]);
				}catch(e2){
					/*squelch IE internal error, see http://trac.dojotoolkit.org/ticket/1984 */
				}
			}
		}, 0);
	},

Note the setTimeout wrapper around the part that actually does the node.focus() and such.

With that applied, IE7 no longer crashes. The setTimeout assured the focus would occur outside an event handler.

I don't think that's a good fix, though. It would be better to locate the keybind to ESC and make sure it is delaying the focus shift until outside the handler.

comment:2 Changed 10 years ago by Jared Jurkiewicz

As an aside, I verified it was not the changes to the moveToBookmark/getBookmark code. That code is never called as there is no bookmark to restore. I had tracked it down as best I could to that focusNode.focus(); call.

So ... my theory is that the esc keybind for dialog changed in 1.4, and now invokes focus within the key event handler, and is why this is now showing up.

comment:3 Changed 10 years ago by bill

Milestone: tbd1.4
Status: newassigned
Summary: Tooltip Dialog: crash in IE when open dialog using space key and close via escape keyTooltipDialog: crash in IE when open dialog using space key and close via escape key

Cool, thanks for tracking down that it's a problem with focus() called from an event handler. I've seen problems w/that too, although never a crash. I'll work on it.

I'm guessing this is a regression?

comment:4 Changed 10 years ago by bill

In both 1.3 and 1.4 the !TooltipDialog_onKey() handler calls focus() w/out a timeout

1.3: TooltipDialog._onKey() --> TooltipDialog._closeDropDown() --> TooltipDialog.focus() --> dijit.focus() 1.4: TooltipDialog._onKey() --> !_HasDropDwon.closeDropDown() --> TooltipDialog.focus() --> dijit.focus()

Actually, lots of code in dijit calls focus w/out a timeout, like the left/right arrow key navigation code in _KeyNavContainer. Not to mention code for handling mouse clicks.

OTOH, I know that sometimes there are issues, which is why ColorPalette._navigateByKey() has a setTimeout().

I thought the problem was that HasDropDown?._closeDropDown() is hiding the node with the focus. But changing the order (focusing the button first, then closing the dialog) didn't fix the crash.

I put a setTimeout() into _onKey() which fixes the crash. In the future we might want a more general solution for handling events.

comment:5 Changed 10 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [20907]) Use setTimeout() to avoid crash on IE. The handler for ESC was both changing focus and changing the DOM (ie, closing the popup), which was just too much for IE to take.

Fixes #10396 !strict.

Note: See TracTickets for help on using tickets.