Skip to main content

Search and Top Navigation

#9241 closed bug (fixed)

Opened April 23, 2013 06:16AM UTC

Closed November 20, 2013 03:04PM UTC

Last modified November 20, 2013 03:07PM UTC

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

Reported by: creage Owned by:
Priority: minor Milestone: 1.11.0
Component: ui.dialog Version: 1.10.2
Keywords: Cc:
Blocked by: Blocking:
Description

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.

Attachments (0)
Change History (16)

Changed April 23, 2013 06:22AM UTC by creage comment:1

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

Changed April 23, 2013 12:45PM UTC by scottgonzalez comment:2

resolution: → duplicate
status: newclosed

Duplicate of #9097.

Changed April 23, 2013 05:30PM UTC by creage comment:3

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

Changed April 23, 2013 05:36PM UTC by scottgonzalez comment:4

resolution: duplicate
status: closedreopened

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

Changed April 23, 2013 05:36PM UTC by scottgonzalez comment:5

status: reopenedopen

Changed April 23, 2013 05:43PM UTC by creage comment:6

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

Changed September 30, 2013 11:02AM UTC by jzaefferer comment:7

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?

Changed September 30, 2013 04:59PM UTC by creage comment:8

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;
}

Changed October 18, 2013 02:27PM UTC by jzaefferer comment:9

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.

Changed October 18, 2013 04:45PM UTC by scottgonzalez comment:10

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.

Changed October 18, 2013 10:17PM UTC by jzaefferer comment:11

Valid point. Any ideas what we could do instead?

Changed November 14, 2013 08:27PM UTC by scottgonzalez comment:12

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().

Changed November 15, 2013 03:51PM UTC by jzaefferer comment:13

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.

Changed November 16, 2013 11:22AM UTC by jzaefferer comment:14

summary: UI dialog inheritance causes undefined property '_focusTabbable' in IE9Dialog: UI dialog inheritance causes undefined property '_focusTabbable' in IE9

Changed November 20, 2013 03:04PM UTC by Jörn Zaefferer comment:15

resolution: → fixed
status: openclosed

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

Changed November 20, 2013 03:07PM UTC by scottgonzalez comment:16

milestone: none1.11.0