Dialog: UI dialog inheritance causes undefined property '_focusTabbable' in IE9

[creage]

1. Create a new widget inherited from UIDialog.
2. Init two instances of that widget, and try to open them from code.
3. Close first one from code.

Actual: IE9 trows an error of undefined property _focusTabbable. Expected: Inherited dialogs work without errors.

See ​http://jsfiddle.net/NDFCd/4/

Tested on IE9 having Compatibility Mode disabled.

[creage]

Seems like it only fails when you do these steps:

1. Create a new widget inherited from UIDialog.
2. Init an instance of that widget, open it.
3. Init an instance of standard UIDialog, open it
4. Now close the inherited instance, created on step #2

[Scott González]

Duplicate of #9097.

[creage]

It is not a duplicate, since #9097 is fixed, but this bug is still reproducible.

[Scott González]

I don't know why this would be browser-specific.

[creage]

It is reproducible on IE10 as well - seems like MS-style issue.

[Jörn Zaefferer]

This is the line throwing the error: ​https://github.com/jquery/jquery-ui/blob/1.10.3/ui/jquery.ui.dialog.js#L722 via a jsbin that includes a non-minified jQuery UI version: ​http://jsbin.com/iziraje/1

Considering the context, its not surprising that this only fails in IE (I've tested IE8). In Chrome that line isn't executed when loading the test page.

The problem is the call to .data( widgetFullName ). An inherited dialog instance stores its data under a different key, here "sj-blocker", so that access fails.

This kind of namespacing problem isn't exaclty new. For grid dataviews, we ended up forcing consistent event prefixes, to make events reliable even in the context of inheritance, e.g. ​https://github.com/jquery/jquery-ui/blob/0d60241a62bf4511708b2ab2a1a840296da31d5c/grid-spf/slideshow-dataview.js#L14 Maybe we need something comparable here to get a little closer to actual polymorphism?

[creage]

Personally I'm fixing this issue by rewriting _allowInteraction method

_allowInteraction: function (event) {
			
	var result = this._super(event);
			
	if (!result) { // replicate ui-dialog instance to fix jQueryUI
				
		var tabbableWidgetInstance = $('.ui-dialog:visible:last .ui-dialog-content'),
			inst = tabbableWidgetInstance.data(this.widgetFullName);
				
		if (!inst) {
			tabbableWidgetInstance.data(this.widgetFullName, tabbableWidgetInstance.data('ui-dialog'));
		}
	}

	return result;
}

[Jörn Zaefferer]

I've been thinking about this some more. One possibility is to also store inherited widgets under the widgetFullName of their parent. So all instances of dialog, inherited or not, would be stored as ui-dialog. That would allow dialog itself to access other dialog instances with .data( "ui-dialog" ). Storing both would still allow access to the actual name. That would fix this particular issue and would improve polymorphism in general, since you could do this:

$.widget( "sj.customDialog", $.ui.dialog, {
  options: {
    autoOpen: false
  }
}):
var div =$("div").customDialog();
div.dialog("open");

On the implementation side:

• in $.widget, if base is not $.Widget, find the base that inherits from $.Widget (in case of deeper inheritance chains) and store its widgetFullName as baseFullName on the constructor.prototype
• in $.widget.bridge, if object.prototype.baseFullName exists, call $.data( this, object.prototype.baseFullName, instance ), after the other $.data call, storing the same instance (reuse existing instance variable)
• in dialog, use .data( "ui-dialog" ) instead of .data( this.widgetFullName )

Note that there's no other change needed to support method calls on the parent type. Pretty nice, unless I'm missing something.

Also we should extend the jQuery.widget documentation or learn articles with details on polymorphism through the widget bridge.

[Scott González]

That's not a valid approach, since it won't work with things like elem.draggable().resizable() where both draggable and resizable have a common base.

[Jörn Zaefferer]

Valid point. Any ideas what we could do instead?

[Scott González]

We can track the dialog instance that was most recently interacted with, and just call _focusTabbable() on that instance. If we go with that, we should wait for ​https://github.com/jquery/jquery-ui/pull/1108 to land first, since we can leverage _trackFocus().

[Jörn Zaefferer]

Discussed this with Scott: In _trackFocus() we store the active instance via this.document.data("ui-dialog-recent", ...);. That data attribute is an array. If the instance already exists in there, we remove it. Then we add it at the end.

In the overlay/focusin handler, we call _focusTabbable on the last element in the array. There must be at least one element in the array.

We also remove the instance from that array when when closing a dialog.

[Jörn Zaefferer]

Dialog: Keep track of instances to focus when elements outside the dialog get focus. Works with inheritance. Adds tests for both. Fixes #9241 - Dialog: UI dialog inheritance causes undefined property '_focusTabbable' in IE9

Changeset: 1096f19f37d6075328509d62a4c2c6d6a53d4b37