From 2a887e43213c4dbb21509b670cf5dc8ac2c67573 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Thu, 25 Oct 2012 20:30:10 -0400 Subject: [PATCH 01/43] Dialog: Improve accessibilty - add an aria-describedby attribute on the dialog if there is nothing yet in the dialog content. Partial fix for: --- ui/jquery.ui.dialog.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index 07c5c7cb6af..892dd72b099 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -156,6 +156,15 @@ $.widget("ui.dialog", { "aria-labelledby": uiDialogTitle.attr( "id" ) }); + // We assume that any existing aria-describedby attribute means + // that the dialog content is marked up properly + // otherwise we brute force the content as the description + if ( !this.element.find( "[aria-describedby]" ).length ) { + uiDialog.attr({ + "aria-describedby": this.element.uniqueId().attr( "id" ) + }); + } + uiDialogTitlebar.find( "*" ).add( uiDialogTitlebar ).disableSelection(); this._hoverable( uiDialogTitlebarClose ); this._focusable( uiDialogTitlebarClose ); @@ -205,6 +214,7 @@ $.widget("ui.dialog", { } this.uiDialog.hide(); this.element + .removeUniqueId() .removeClass( "ui-dialog-content ui-widget-content" ) .hide() .appendTo( "body" ); From 8ee8046c029354501bc6d1690b3ac84edf23efd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Fri, 26 Oct 2012 10:59:41 -0400 Subject: [PATCH 02/43] Dialog: Keep focus inside modal dialog, by handling focus events on elements outside of it --- ui/jquery.ui.dialog.js | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index 892dd72b099..ed1e55bebf4 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -282,8 +282,7 @@ $.widget("ui.dialog", { return; } - var hasFocus, - options = this.options, + var options = this.options, uiDialog = this.uiDialog; this.opener = $( this.document[ 0 ].activeElement ); @@ -294,22 +293,26 @@ $.widget("ui.dialog", { this.moveToTop( null, true ); this._show( uiDialog, options.show ); + this._focusTabbable(); + + this._isOpen = true; + this._trigger( "open" ); + this._trigger( "focus" ); + + return this; + }, + + _focusTabbable: function() { // set focus to the first tabbable element in the content area or the first button // if there are no tabbable elements, set focus on the dialog itself - hasFocus = this.element.find( ":tabbable" ); + var hasFocus = this.element.find( ":tabbable" ); if ( !hasFocus.length ) { hasFocus = this.uiDialogButtonPane.find( ":tabbable" ); if ( !hasFocus.length ) { - hasFocus = uiDialog; + hasFocus = this.uiDialog; } } hasFocus.eq( 0 ).focus(); - - this._isOpen = true; - this._trigger( "open" ); - this._trigger( "focus" ); - - return this; }, _keepFocus: function( event ) { @@ -318,7 +321,7 @@ $.widget("ui.dialog", { isActive = this.uiDialog[ 0 ] === activeElement || $.contains( this.uiDialog[ 0 ], activeElement ); if ( !isActive ) { - this.uiDialog.focus(); + this._focusTabbable(); } } event.preventDefault(); @@ -659,6 +662,22 @@ $.extend( $.ui.dialog.overlay, { // reuse old instances due to IE memory leak with alpha transparency (see #5185) oldInstances: [], create: function( dialog ) { + if ( this.instances.length === 0 ) { + // prevent use of anchors and inputs + // we use a setTimeout in case the overlay is created from an + // event that we're going to be cancelling (see #2804) + setTimeout(function() { + // handle $(el).dialog().dialog('close') (see #4065) + if ( $.ui.dialog.overlay.instances.length ) { + $( document ).bind( "focusin.dialog-overlay", function( event ) { + if ( !$( event.target ).closest( ".ui-dialog").length ) { + event.preventDefault(); + $( ".ui-dialog:visible:last .ui-dialog-content" ).data( "ui-dialog" )._focusTabbable(); + } + }); + } + }, 1 ); + } var $el = ( this.oldInstances.pop() || $( "
" ).addClass( "ui-widget-overlay ui-front" ) ); From 0a25b2c4483b799e904f9dcd2100e27c488c8f20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Fri, 26 Oct 2012 11:08:35 -0400 Subject: [PATCH 03/43] Dialog: move to top when opening again, and focus as if opened from scratch. --- ui/jquery.ui.dialog.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index ed1e55bebf4..c25b28f7180 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -279,6 +279,8 @@ $.widget("ui.dialog", { open: function() { if ( this._isOpen ) { + this.moveToTop( null, true ); + this._focusTabbable(); return; } From 2062a18db6eb1544c2f28d68b6f55d60eef0fd65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Sat, 27 Oct 2012 17:07:13 +0200 Subject: [PATCH 04/43] Dialog: Inline code review --- ui/jquery.ui.dialog.js | 58 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 53 insertions(+), 5 deletions(-) diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index c25b28f7180..7545a8adf4d 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -68,12 +68,25 @@ $.widget("ui.dialog", { resizable: true, show: null, title: "", - width: 300 + width: 300, + + // callbacks + beforeClose: null, + close: null, + drag: null, + dragStart: null, + dragStop: null, + focus: null, + open: null, + resize: null, + resizeStart: null, + resizeStop: null }, _create: function() { this.originalTitle = this.element.attr( "title" ); // #5742 - .attr() might return a DOMElement + // TODO WTF? if ( typeof this.originalTitle !== "string" ) { this.originalTitle = ""; } @@ -81,17 +94,22 @@ $.widget("ui.dialog", { parent: this.element.parent(), index: this.element.parent().children().index( this.element ) }; + // TODO don't overwrite options this.options.title = this.options.title || this.originalTitle; var that = this, options = this.options, + // TODO make this the default for the title option? title = options.title || " ", + // TODO should use this.uiDialog instead uiDialog, + // TODO should use this.uiDialogTitlebar instead uiDialogTitlebar, uiDialogTitlebarClose, uiDialogTitle, uiDialogButtonPane; + // TODO extract into _createWrapper uiDialog = ( this.uiDialog = $( "
" ) ) .addClass( uiDialogClasses + options.dialogClass ) .hide() @@ -115,9 +133,10 @@ $.widget("ui.dialog", { .addClass( "ui-dialog-content ui-widget-content" ) .appendTo( uiDialog ); + // TODO extract this and the next three into a _createTitlebar method uiDialogTitlebar = ( this.uiDialogTitlebar = $( "
" ) ) - .addClass( "ui-dialog-titlebar ui-widget-header " + - "ui-corner-all ui-helper-clearfix" ) + .addClass( "ui-dialog-titlebar ui-widget-header ui-corner-all ui-helper-clearfix" ) + // TODO use _on, call _focusTabbable or _keepFocus .bind( "mousedown", function() { // Dialog isn't getting focus when dragging (#8063) uiDialog.focus(); @@ -144,6 +163,7 @@ $.widget("ui.dialog", { .html( title ) .prependTo( uiDialogTitlebar ); + // TODO extract this one and the next into a _createButtonPane method uiDialogButtonPane = ( this.uiDialogButtonPane = $( "
" ) ) .addClass( "ui-dialog-buttonpane ui-widget-content ui-helper-clearfix" ); @@ -151,11 +171,13 @@ $.widget("ui.dialog", { .addClass( "ui-dialog-buttonset" ) .appendTo( uiDialogButtonPane ); + // TODO move into _createWrapper uiDialog.attr({ role: "dialog", "aria-labelledby": uiDialogTitle.attr( "id" ) }); + // TODO move into _createWrapper // We assume that any existing aria-describedby attribute means // that the dialog content is marked up properly // otherwise we brute force the content as the description @@ -165,7 +187,11 @@ $.widget("ui.dialog", { }); } + // TODO use andSelf() + // TODO get rid of this?! why do we need to disable selection anyway? uiDialogTitlebar.find( "*" ).add( uiDialogTitlebar ).disableSelection(); + + // TODO use button? or at least a button element, so that SPACE works? this._hoverable( uiDialogTitlebarClose ); this._focusable( uiDialogTitlebarClose ); @@ -176,10 +202,14 @@ $.widget("ui.dialog", { this._makeResizable(); } + // TODO merge with _createButtonPane? this._createButtons( options.buttons ); + this._isOpen = false; // prevent tabbing out of dialogs + // TODO move into _createWrapper + // TODO fix formatting this._on( uiDialog, { keydown: function( event ) { if ( event.keyCode !== $.ui.keyCode.TAB ) { return; @@ -217,6 +247,7 @@ $.widget("ui.dialog", { .removeUniqueId() .removeClass( "ui-dialog-content ui-widget-content" ) .hide() + // TODO restore old position directly, instead of appending to body first .appendTo( "body" ); this.uiDialog.remove(); @@ -224,6 +255,7 @@ $.widget("ui.dialog", { this.element.attr( "title", this.originalTitle ); } + // TODO do this before removing the wrapper next = oldPosition.parent.children().eq( oldPosition.index ); // Don't try to place the dialog next to itself (#8613) if ( next.length && next[ 0 ] !== this.element[ 0 ] ) { @@ -244,6 +276,7 @@ $.widget("ui.dialog", { return; } + // TODO fix yoda-if if ( false === this._trigger( "beforeClose", event ) ) { return; } @@ -261,6 +294,7 @@ $.widget("ui.dialog", { $( this.document[ 0 ].activeElement ).blur(); } + // TODO shouldn't _hide restore `this` to the instance? would also help tooltip this._hide( this.uiDialog, this.options.hide, function() { that._trigger( "close", event ); }); @@ -279,11 +313,14 @@ $.widget("ui.dialog", { open: function() { if ( this._isOpen ) { + // TODO don't pass silent flag? should probably trigger focus when moving to top again this.moveToTop( null, true ); + // TODO run this only when dialog wasn't focused? this._focusTabbable(); return; } + // TODO remove useless tmp vars var options = this.options, uiDialog = this.uiDialog; @@ -304,6 +341,7 @@ $.widget("ui.dialog", { return this; }, + // TODO check if dialog already has focus, merge with _keepFocus _focusTabbable: function() { // set focus to the first tabbable element in the content area or the first button // if there are no tabbable elements, set focus on the dialog itself @@ -342,6 +380,7 @@ $.widget("ui.dialog", { this.uiDialogButtonPane.remove(); this.uiButtonSet.empty(); + // TODO use jQuery.isEmptyObject() if ( typeof buttons === "object" && buttons !== null ) { $.each( buttons, function() { return !(hasButtons = true); @@ -363,6 +402,7 @@ $.widget("ui.dialog", { button = $( "", props ) .appendTo( that.uiButtonSet ); if ( $.fn.button ) { + // TODO allow passing through button options button.button(); } }); @@ -408,6 +448,7 @@ $.widget("ui.dialog", { }); }, + // TODO why are handles passed by _setOption?? _makeResizable: function( handles ) { handles = (handles === undefined ? this.options.resizable : handles); var that = this, @@ -472,6 +513,7 @@ $.widget("ui.dialog", { isVisible; if ( position ) { + // TODO we don't support 1.3.2 anymore, clean this mess up // deep extending converts arrays to objects in jQuery <= 1.3.2 :-( // if (typeof position == 'string' || $.isArray(position)) { // myAt = $.isArray(position) ? position : position.split(' '); @@ -551,9 +593,11 @@ $.widget("ui.dialog", { case "dialogClass": uiDialog .removeClass( this.options.dialogClass ) + // TODO why adding uiDialogClasses again? we didn't remove those .addClass( uiDialogClasses + value ); break; case "disabled": + // TODO use toggleClass( "ui-dialog-disabled", value ) if ( value ) { uiDialog.addClass( "ui-dialog-disabled" ); } else { @@ -591,7 +635,8 @@ $.widget("ui.dialog", { } break; case "title": - // convert whatever was passed in o a string, for html() to not throw up + // convert whatever was passed in to a string, for html() to not throw up + // TODO deduplicate this (see _create) $( ".ui-dialog-title", this.uiDialogTitlebar ) .html( "" + ( value || " " ) ); break; @@ -643,8 +688,8 @@ $.widget("ui.dialog", { }); $.extend($.ui.dialog, { + // TODO remove these uuid: 0, - getTitleId: function($el) { var id = $el.attr( "id" ); if ( !id ) { @@ -654,17 +699,20 @@ $.extend($.ui.dialog, { return "ui-dialog-title-" + id; }, + // TODO move to dialog instance method overlay: function( dialog ) { this.$el = $.ui.dialog.overlay.create( dialog ); } }); +// TODO get rid of instance list, at least the oldInstance stuff, and inline as dialog methods $.extend( $.ui.dialog.overlay, { instances: [], // reuse old instances due to IE memory leak with alpha transparency (see #5185) oldInstances: [], create: function( dialog ) { if ( this.instances.length === 0 ) { + // TODO get rid of the timeout, which should remove the need for the #4065 workaround as well // prevent use of anchors and inputs // we use a setTimeout in case the overlay is created from an // event that we're going to be cancelling (see #2804) From 46327804350874d21c564bc72d7d3c541bef9378 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Fri, 9 Nov 2012 17:17:32 +0100 Subject: [PATCH 05/43] Dialog: Fix yoda-if, remove unnecessary TODOs; add missing callbacks to commons test --- tests/unit/dialog/dialog_common.js | 12 +++++++++++- ui/jquery.ui.dialog.js | 5 +---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/unit/dialog/dialog_common.js b/tests/unit/dialog/dialog_common.js index 2c0e86b46c5..fb29838b706 100644 --- a/tests/unit/dialog/dialog_common.js +++ b/tests/unit/dialog/dialog_common.js @@ -27,6 +27,16 @@ TestHelpers.commonWidgetTests( "dialog", { width: 300, // callbacks - create: null + beforeClose: null, + close: null, + create: null, + drag: null, + dragStart: null, + dragStop: null, + focus: null, + open: null, + resize: null, + resizeStart: null, + resizeStop: null } }); diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index 7545a8adf4d..04b23f2a1d4 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -94,7 +94,6 @@ $.widget("ui.dialog", { parent: this.element.parent(), index: this.element.parent().children().index( this.element ) }; - // TODO don't overwrite options this.options.title = this.options.title || this.originalTitle; var that = this, options = this.options, @@ -276,8 +275,7 @@ $.widget("ui.dialog", { return; } - // TODO fix yoda-if - if ( false === this._trigger( "beforeClose", event ) ) { + if ( this._trigger( "beforeClose", event ) === false ) { return; } @@ -294,7 +292,6 @@ $.widget("ui.dialog", { $( this.document[ 0 ].activeElement ).blur(); } - // TODO shouldn't _hide restore `this` to the instance? would also help tooltip this._hide( this.uiDialog, this.options.hide, function() { that._trigger( "close", event ); }); From b6cefc797e1f4cbede6c0d69b0abf993244ea66c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Fri, 9 Nov 2012 17:26:52 +0100 Subject: [PATCH 06/43] Dialog: Remove deprecated disableSelection() usage. --- ui/jquery.ui.dialog.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index 04b23f2a1d4..0ed52934572 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -186,10 +186,6 @@ $.widget("ui.dialog", { }); } - // TODO use andSelf() - // TODO get rid of this?! why do we need to disable selection anyway? - uiDialogTitlebar.find( "*" ).add( uiDialogTitlebar ).disableSelection(); - // TODO use button? or at least a button element, so that SPACE works? this._hoverable( uiDialogTitlebarClose ); this._focusable( uiDialogTitlebarClose ); From 4e03321fd7cb41ba43a945982de2cb636fd7c70e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Fri, 9 Nov 2012 17:39:59 +0100 Subject: [PATCH 07/43] Dialog: Trigger focus event when dialog is moved to top. --- ui/jquery.ui.dialog.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index 0ed52934572..62813940749 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -306,8 +306,7 @@ $.widget("ui.dialog", { open: function() { if ( this._isOpen ) { - // TODO don't pass silent flag? should probably trigger focus when moving to top again - this.moveToTop( null, true ); + this.moveToTop( null ); // TODO run this only when dialog wasn't focused? this._focusTabbable(); return; From 324d54dd3283bb83c84de238927724ce8e29efcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Fri, 9 Nov 2012 17:42:11 +0100 Subject: [PATCH 08/43] Dialog: Use $.isEmptyObject() to check if there a button-option properties --- ui/jquery.ui.dialog.js | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index 62813940749..f536120d572 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -365,20 +365,13 @@ $.widget("ui.dialog", { }, _createButtons: function( buttons ) { - var that = this, - hasButtons = false; + var that = this; // if we already have a button pane, remove it this.uiDialogButtonPane.remove(); this.uiButtonSet.empty(); - // TODO use jQuery.isEmptyObject() - if ( typeof buttons === "object" && buttons !== null ) { - $.each( buttons, function() { - return !(hasButtons = true); - }); - } - if ( hasButtons ) { + if ( !$.isEmptyObject( buttons ) ) { $.each( buttons, function( name, props ) { var button, click; props = $.isFunction( props ) ? From f7d3a51589b30fd1be59339ca65b0e4bc6cfac8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Fri, 9 Nov 2012 17:48:25 +0100 Subject: [PATCH 09/43] Dialog: Only add the new dialogClass, not the base classes when changing the option. --- ui/jquery.ui.dialog.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index f536120d572..ab32219c254 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -578,8 +578,7 @@ $.widget("ui.dialog", { case "dialogClass": uiDialog .removeClass( this.options.dialogClass ) - // TODO why adding uiDialogClasses again? we didn't remove those - .addClass( uiDialogClasses + value ); + .addClass( value ); break; case "disabled": // TODO use toggleClass( "ui-dialog-disabled", value ) From 6edce867339c5808eda428fa2566aa40da4b0202 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Fri, 9 Nov 2012 18:26:30 +0100 Subject: [PATCH 10/43] Dialog: Remove attroperty workaround for title attribute. Make the default null, as it should be. No back-compat, as the behaviour doesn't change. --- tests/unit/dialog/dialog_common.js | 2 +- tests/unit/dialog/dialog_options.js | 42 +++++++++++++++++------------ ui/jquery.ui.dialog.js | 9 ++----- 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/tests/unit/dialog/dialog_common.js b/tests/unit/dialog/dialog_common.js index fb29838b706..47fff101352 100644 --- a/tests/unit/dialog/dialog_common.js +++ b/tests/unit/dialog/dialog_common.js @@ -23,7 +23,7 @@ TestHelpers.commonWidgetTests( "dialog", { }, resizable: true, show: null, - title: '', + title: null, width: 300, // callbacks diff --git a/tests/unit/dialog/dialog_options.js b/tests/unit/dialog/dialog_options.js index 33123031898..899dc1df670 100644 --- a/tests/unit/dialog/dialog_options.js +++ b/tests/unit/dialog/dialog_options.js @@ -415,37 +415,45 @@ test("resizable", function() { el.remove(); }); -test("title", function() { - expect(9); +test( "title", function() { + expect( 11 ); function titleText() { - return el.dialog('widget').find(".ui-dialog-title").html(); + return el.dialog('widget').find( ".ui-dialog-title" ).html(); } - var el = $('
').dialog(); + var el = $( '
' ).dialog(); // some browsers return a non-breaking space and some return " " // so we generate a non-breaking space for comparison - equal(titleText(), $( " " ).html(), "[default]"); - equal(el.dialog("option", "title"), "", "option not changed"); + equal( titleText(), $( " " ).html(), "[default]" ); + equal( el.dialog( "option", "title" ), null, "option not changed" ); + el.remove(); + + el = $( '
' ).dialog(); + equal( titleText(), "foo", "title in element attribute" ); + equal( el.dialog( "option", "title"), "foo", "option updated from attribute" ); el.remove(); - el = $('
').dialog(); - equal(titleText(), "foo", "title in element attribute"); - equal(el.dialog("option", "title"), "foo", "option updated from attribute"); + el = $( '
' ).dialog({ title: 'foo' }); + equal( titleText(), "foo", "title in init options" ); + equal( el.dialog("option", "title"), "foo", "opiton set from options hash" ); el.remove(); - el = $('
').dialog({ title: 'foo' }); - equal(titleText(), "foo", "title in init options"); - equal(el.dialog("option", "title"), "foo", "opiton set from options hash"); + el = $( '
' ).dialog({ title: 'bar' }); + equal( titleText(), "bar", "title in init options should override title in element attribute" ); + equal( el.dialog("option", "title"), "bar", "opiton set from options hash" ); el.remove(); - el = $('
').dialog({ title: 'bar' }); - equal(titleText(), "bar", "title in init options should override title in element attribute"); - equal(el.dialog("option", "title"), "bar", "opiton set from options hash"); + el = $( '
' ).dialog().dialog( 'option', 'title', 'foo' ); + equal( titleText(), 'foo', 'title after init' ); el.remove(); - el = $('
').dialog().dialog('option', 'title', 'foo'); - equal(titleText(), 'foo', 'title after init'); + // make sure attroperties are properly ignored - #5742 - .attr() might return a DOMElement + el = $( '
' ).dialog(); + // some browsers return a non-breaking space and some return " " + // so we get the text to normalize to the actual non-breaking space + equal( titleText(), $( " " ).html(), "[default]" ); + equal( el.dialog( "option", "title" ), null, "option not changed" ); el.remove(); }); diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index ab32219c254..451203cae52 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -67,7 +67,7 @@ $.widget("ui.dialog", { }, resizable: true, show: null, - title: "", + title: null, width: 300, // callbacks @@ -85,16 +85,11 @@ $.widget("ui.dialog", { _create: function() { this.originalTitle = this.element.attr( "title" ); - // #5742 - .attr() might return a DOMElement - // TODO WTF? - if ( typeof this.originalTitle !== "string" ) { - this.originalTitle = ""; - } + this.options.title = this.options.title || this.originalTitle; this.oldPosition = { parent: this.element.parent(), index: this.element.parent().children().index( this.element ) }; - this.options.title = this.options.title || this.originalTitle; var that = this, options = this.options, From 0848040d3ee041e443e2492c18a5a69c78ab9c12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Fri, 9 Nov 2012 18:44:29 +0100 Subject: [PATCH 11/43] Dialog: Focus tabbable only when dialog lost focus before. --- tests/unit/dialog/dialog_events.js | 45 ++++++++++++++++++++++++++++++ ui/jquery.ui.dialog.js | 23 +++++++++------ 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/tests/unit/dialog/dialog_events.js b/tests/unit/dialog/dialog_events.js index ee7a8bcb3e3..eeb17eb93bc 100644 --- a/tests/unit/dialog/dialog_events.js +++ b/tests/unit/dialog/dialog_events.js @@ -39,6 +39,51 @@ test("open", function() { el.remove(); }); + +test( "focus", function() { + expect( 5 ); + var el, other; + el = $("#dialog1").dialog({ + autoOpen: false + }); + other = $("#dialog2").dialog({ + autoOpen: false + }); + + el.one( "dialogopen", function() { + ok( true, "open, just once" ); + }); + el.one( "dialogfocus", function() { + ok( true, "focus on open" ); + }); + other.dialog( "open" ); + + el.one( "dialogfocus", function() { + ok( true, "when opening and already open and wasn't on top" ); + }); + other.dialog( "open" ); + el.dialog( "open" ); + + el.one( "dialogfocus", function() { + ok( true, "when calling moveToTop and wasn't on top" ); + }); + other.dialog( "moveToTop" ); + el.dialog( "moveToTop" ); + + el.bind( "dialogfocus", function() { + ok( true, "when mousedown anywhere on the dialog and it wasn't on top" ); + }); + other.dialog( "moveToTop" ); + el.trigger( "mousedown" ); + + // triggers just once when already on top + el.dialog( "open" ); + el.dialog( "moveToTop" ); + el.trigger( "mousedown" ); + + el.add( other ).remove(); +}); + test("dragStart", function() { expect(9); diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index 451203cae52..b860a8b1880 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -117,7 +117,9 @@ $.widget("ui.dialog", { } }) .mousedown(function( event ) { - that.moveToTop( event ); + if ( that._moveToTop( event ) ) { + that._focusTabbable(); + } }) .appendTo( this.document[ 0 ].body ); @@ -292,18 +294,23 @@ $.widget("ui.dialog", { return this._isOpen; }, - moveToTop: function( event, silent ) { - var moved = this.uiDialog.nextAll( ":visible" ).insertBefore( this.uiDialog ); - if ( !silent && moved.length ) { + moveToTop: function() { + this._moveToTop(); + }, + + _moveToTop: function( event, silent ) { + var moved = !!this.uiDialog.nextAll( ":visible" ).insertBefore( this.uiDialog ).length; + if ( !silent && moved ) { this._trigger( "focus", event ); } + return moved; }, open: function() { if ( this._isOpen ) { - this.moveToTop( null ); - // TODO run this only when dialog wasn't focused? - this._focusTabbable(); + if ( this._moveToTop() ) { + this._focusTabbable(); + } return; } @@ -316,7 +323,7 @@ $.widget("ui.dialog", { this._size(); this._position( options.position ); this.overlay = options.modal ? new $.ui.dialog.overlay( this ) : null; - this.moveToTop( null, true ); + this._moveToTop( null, true ); this._show( uiDialog, options.show ); this._focusTabbable(); From 83a9f219bf40ff834d660020bbfa7de550e48a0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Thu, 15 Nov 2012 22:29:24 +0100 Subject: [PATCH 12/43] Dialog: Use button widget for close button (was already listed as dependency) --- demos/dialog/animated.html | 1 + demos/dialog/default.html | 1 + demos/dialog/modal-confirmation.html | 1 + demos/dialog/modal-form.html | 7 ++++--- demos/dialog/modal-message.html | 1 + demos/dialog/modal.html | 1 + tests/unit/dialog/dialog.html | 1 + tests/unit/dialog/dialog_options.js | 8 ++++---- tests/visual/dialog/animated.html | 1 + tests/visual/dialog/complex-dialogs.html | 1 + tests/visual/dialog/form.html | 1 + tests/visual/dialog/performance.html | 1 + themes/base/jquery.ui.dialog.css | 12 ++--------- ui/jquery.ui.dialog.js | 26 +++++++++++------------- 14 files changed, 32 insertions(+), 31 deletions(-) diff --git a/demos/dialog/animated.html b/demos/dialog/animated.html index 8e1daf131f3..061396548fb 100644 --- a/demos/dialog/animated.html +++ b/demos/dialog/animated.html @@ -11,6 +11,7 @@ + diff --git a/demos/dialog/default.html b/demos/dialog/default.html index 214f41d0d0e..777921e4877 100644 --- a/demos/dialog/default.html +++ b/demos/dialog/default.html @@ -11,6 +11,7 @@ + + + @@ -86,10 +87,10 @@ if ( bValid ) { $( "#users tbody" ).append( "" + - "" + name.val() + "" + - "" + email.val() + "" + + "" + name.val() + "" + + "" + email.val() + "" + "" + password.val() + "" + - "" ); + "" ); $( this ).dialog( "close" ); } }, diff --git a/demos/dialog/modal-message.html b/demos/dialog/modal-message.html index 60b7c3e1516..19fc0e829bc 100644 --- a/demos/dialog/modal-message.html +++ b/demos/dialog/modal-message.html @@ -12,6 +12,7 @@ + + + diff --git a/tests/visual/dialog/complex-dialogs.html b/tests/visual/dialog/complex-dialogs.html index 6399d8a4b0e..2b9a0d3a6b8 100644 --- a/tests/visual/dialog/complex-dialogs.html +++ b/tests/visual/dialog/complex-dialogs.html @@ -11,6 +11,7 @@ + diff --git a/tests/visual/dialog/form.html b/tests/visual/dialog/form.html index 867991911ae..ed70ec69ba7 100644 --- a/tests/visual/dialog/form.html +++ b/tests/visual/dialog/form.html @@ -11,6 +11,7 @@ + diff --git a/tests/visual/dialog/performance.html b/tests/visual/dialog/performance.html index 937b9464f0e..8f63d6a69d8 100644 --- a/tests/visual/dialog/performance.html +++ b/tests/visual/dialog/performance.html @@ -11,6 +11,7 @@ + @@ -91,13 +96,13 @@

WHAT: A modal dialog opening another modal dialog (including a datepicker), opening a non-modal dialog (including an autocomplete with tooltip applied). A regular link on the page, outside of the dialogs.

EXPECTED: As long as a modal dialog is open, focus stays within the dialogs. Both mouse and keyboard interactions are captured and restricted to the dialog. When the nested modal dialog is open, the first modal dialog can't be interacted with, until the nested dialog is closed. When the third dialog is open (not modal), switching between the two dialogs is possible, both can be interacted with.

-Outside link

This is the default dialog which is useful for displaying information. The dialog window can be moved, resized and closed with the 'x' icon.

+

@@ -110,5 +115,7 @@
+Outside link + From 299681e8f0896cd0448fa08003bb7b733adc489f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Fri, 16 Nov 2012 19:46:01 +0100 Subject: [PATCH 30/43] Dialog: Cleanup in ticket tests: TODO to merge one test, fix whitespace --- tests/unit/dialog/dialog_tickets.js | 59 +++++++++++++++-------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/tests/unit/dialog/dialog_tickets.js b/tests/unit/dialog/dialog_tickets.js index 3055c5fc1ae..655f1e4451e 100644 --- a/tests/unit/dialog/dialog_tickets.js +++ b/tests/unit/dialog/dialog_tickets.js @@ -71,24 +71,24 @@ test("#5184: isOpen in dialogclose event is true", function() { }); test("#5531: dialog width should be at least minWidth on creation", function () { - expect( 4 ); - var el = $('
').dialog({ - width: 200, - minWidth: 300 - }); - - equal(el.dialog('option', 'width'), 300, "width is minWidth"); - el.dialog('option', 'width', 200); - equal(el.dialog('option', 'width'), 300, "width unchanged when set to < minWidth"); - el.dialog('option', 'width', 320); - equal(el.dialog('option', 'width'), 320, "width changed if set to > minWidth"); - el.remove(); - - el = $('
').dialog({ - minWidth: 300 - }); - ok(el.dialog('option', 'width') >= 300, "width is at least 300"); - el.remove(); + expect( 4 ); + var el = $('
').dialog({ + width: 200, + minWidth: 300 + }); + + equal(el.dialog('option', 'width'), 300, "width is minWidth"); + el.dialog('option', 'width', 200); + equal(el.dialog('option', 'width'), 300, "width unchanged when set to < minWidth"); + el.dialog('option', 'width', 320); + equal(el.dialog('option', 'width'), 320, "width changed if set to > minWidth"); + el.remove(); + + el = $('
').dialog({ + minWidth: 300 + }); + ok(el.dialog('option', 'width') >= 300, "width is at least 300"); + el.remove(); }); @@ -108,20 +108,21 @@ test("#6137: dialog('open') causes form elements to reset on IE7", function() { }); test("#6645: Missing element not found check in overlay", function(){ - expect(2); - var d1 = $('
Dialog 1
').dialog({modal: true}), - d2 = $('
Dialog 2
').dialog({modal: true, close: function(){ d2.remove(); }}); - - equal($.ui.dialog.overlay.instances.length, 2, 'two overlays created'); - d2.dialog('close'); - equal($.ui.dialog.overlay.instances.length, 1, 'one overlay remains after closing the 2nd overlay'); - d1.add(d2).remove(); + expect(2); + var d1 = $('
Dialog 1
').dialog({modal: true}), + d2 = $('
Dialog 2
').dialog({modal: true, close: function(){ d2.remove(); }}); + + equal($.ui.dialog.overlay.instances.length, 2, 'two overlays created'); + d2.dialog('close'); + equal($.ui.dialog.overlay.instances.length, 1, 'one overlay remains after closing the 2nd overlay'); + d1.add(d2).remove(); }); +// TODO merge this with the main destroy test test("#4980: Destroy should place element back in original DOM position", function(){ - expect( 2 ); - var container = $('
'), - modal = container.find('#modal'); + expect( 2 ); + var container = $('
'), + modal = container.find('#modal'); modal.dialog(); ok(!$.contains(container[0], modal[0]), 'dialog should move modal element to outside container element'); modal.dialog('destroy'); From b27db7e3b9a857b8f0890e91ae9c8a2d33bf9919 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Fri, 16 Nov 2012 20:24:57 +0100 Subject: [PATCH 31/43] Dialog: Extend autofocus, starting with [autofocus], then :tabbable content, then buttonpane, then close button, then dialog. Fixes #4731 - Dialog: Add option to set which element gains focus on open --- tests/unit/dialog/dialog_core.js | 42 ++++++++++++++++++++++++ tests/visual/dialog/complex-dialogs.html | 2 +- ui/jquery.ui.dialog.js | 22 +++++++++---- 3 files changed, 58 insertions(+), 8 deletions(-) diff --git a/tests/unit/dialog/dialog_core.js b/tests/unit/dialog/dialog_core.js index b36f6204fc5..31b245a61f1 100644 --- a/tests/unit/dialog/dialog_core.js +++ b/tests/unit/dialog/dialog_core.js @@ -17,6 +17,8 @@ test("title id", function() { el.remove(); }); +// TODO test for aria-describedby +// add only when the attribute isn't anywhere yet test("ARIA", function() { expect(4); @@ -42,4 +44,44 @@ test("widget method", function() { deepEqual(dialog.parent()[0], dialog.dialog("widget")[0]); }); +test( "focus tabbable", function() { + expect( 5 ); + var el, + options = { + buttons: [{ + text: "Ok", + click: $.noop + }] + }; + + // 1. first element inside the dialog matching [autofocus] + el = $( "
" ).dialog( options ); + equal( document.activeElement, el.find( "input" )[ 1 ] ); + el.remove(); + + // 2. tabbable element inside the content element + el = $( "
" ).dialog( options ); + equal( document.activeElement, el.find( "input" )[ 0 ] ); + el.remove(); + + // 3. tabbable element inside the buttonpane + el = $( "
text
" ).dialog( options ); + equal( document.activeElement, el.dialog( "widget" ).find( ".ui-dialog-buttonpane button" )[ 0 ] ); + el.remove(); + + // 4. the close button + el = $( "
text
" ).dialog(); + equal( document.activeElement, el.dialog( "widget" ).find( ".ui-dialog-titlebar .ui-dialog-titlebar-close" )[ 0 ] ); + el.remove(); + + // 5. the dialog itself + el = $( "
text
" ).dialog({ + autoOpen: false + }); + el.dialog( "widget" ).find( ".ui-dialog-titlebar-close" ).hide(); + el.dialog( "open" ); + equal( document.activeElement, el.parent()[ 0 ] ); + el.remove(); +}); + })(jQuery); diff --git a/tests/visual/dialog/complex-dialogs.html b/tests/visual/dialog/complex-dialogs.html index 03bb160fb64..5c2e1d8a12c 100644 --- a/tests/visual/dialog/complex-dialogs.html +++ b/tests/visual/dialog/complex-dialogs.html @@ -107,7 +107,7 @@

Date:

-

+

diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index 0033898234a..c48d1a804e1 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -224,15 +224,24 @@ $.widget("ui.dialog", { return this; }, - // TODO check if dialog already has focus, merge with _keepFocus _focusTabbable: function() { - // set focus to the first tabbable element in the content area or the first button - // if there are no tabbable elements, set focus on the dialog itself - var hasFocus = this.element.find( ":tabbable" ); + // set focus to the first match: + // 1. first element inside the dialog matching [autofocus] + // 2. tabbable element inside the content element + // 3. tabbable element inside the buttonpane + // 4. the close button + // 5. the dialog itself + var hasFocus = this.element.find( "[autofocus]" ); if ( !hasFocus.length ) { - hasFocus = this.uiDialogButtonPane.find( ":tabbable" ); + hasFocus = this.element.find( ":tabbable" ); if ( !hasFocus.length ) { - hasFocus = this.uiDialog; + hasFocus = this.uiDialogButtonPane.find( ":tabbable" ); + if ( !hasFocus.length ) { + hasFocus = this.uiDialogTitlebarClose.filter( ":tabbable" ); + if ( !hasFocus.length ) { + hasFocus = this.uiDialog; + } + } } } hasFocus.eq( 0 ).focus(); @@ -316,7 +325,6 @@ $.widget("ui.dialog", { .prependTo( this.uiDialog ); this._on( this.uiDialogTitlebar, { mousedown: function() { - // TODO call _focusTabbable or _keepFocus // Dialog isn't getting focus when dragging (#8063) this.uiDialog.focus(); } From 0be97bf8c01394cd68134b104bcbf30b27859531 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Sat, 17 Nov 2012 11:31:57 +0100 Subject: [PATCH 32/43] Dialog: Removed broken disabled option from dialog, defuse disable/enable methods. Disabling dialogs is not supported. --- tests/unit/dialog/dialog_methods.js | 32 ++++++----------------------- ui/jquery.ui.dialog.js | 7 +++++++ 2 files changed, 13 insertions(+), 26 deletions(-) diff --git a/tests/unit/dialog/dialog_methods.js b/tests/unit/dialog/dialog_methods.js index a4959bf8588..e5275c7a4fe 100644 --- a/tests/unit/dialog/dialog_methods.js +++ b/tests/unit/dialog/dialog_methods.js @@ -47,32 +47,12 @@ test("destroy", function() { }); }); -test("enable", function() { - expect( 3 ); - - var el, - expected = $('
').dialog(), - actual = expected.dialog('enable'); - equal(actual, expected, 'enable is chainable'); - - el = $('
').dialog({ disabled: true }); - el.dialog('enable'); - equal(el.dialog('option', 'disabled'), false, 'enable method sets disabled option to false'); - ok(!el.dialog('widget').hasClass('ui-dialog-disabled'), 'enable method removes ui-dialog-disabled class from ui-dialog element'); -}); - -test("disable", function() { - expect( 3 ); - - var el, - expected = $('
').dialog(), - actual = expected.dialog('disable'); - equal(actual, expected, 'disable is chainable'); - - el = $('
').dialog({ disabled: false }); - el.dialog('disable'); - equal(el.dialog('option', 'disabled'), true, 'disable method sets disabled option to true'); - ok(el.dialog('widget').hasClass('ui-dialog-disabled'), 'disable method adds ui-dialog-disabled class to ui-dialog element'); +test( "enable/disable disabled", function() { + expect( 2 ); + var el = $( "
" ).dialog(); + el.dialog( "disable" ); + equal(el.dialog( "option", "disabled" ), false, "disable method doesn't do anything" ); + ok( !el.dialog( "widget" ).hasClass( "ui-dialog-disabled" ), "disable method doesn't add ui-dialog-disabled class" ); }); test("close", function() { diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index c48d1a804e1..8e5ca66c1fd 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -152,6 +152,9 @@ $.widget("ui.dialog", { return this.uiDialog; }, + disable: $.noop, + enable: $.noop, + close: function( event ) { var that = this; @@ -578,6 +581,10 @@ $.widget("ui.dialog", { .addClass( value ); } + if ( key === "disabled" ) { + return; + } + this._super( key, value ); if ( key === "buttons" ) { From a0310eb091f679c8ba002d57a30b53f8e7ea8ef3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Sat, 17 Nov 2012 12:10:26 +0100 Subject: [PATCH 33/43] Dialog: Move array notation support for position option to backCompat check. Fixes #8824 - Deprecate array notation for position option. --- build/tasks/testswarm.js | 1 + tests/unit/all.html | 1 + tests/unit/dialog/dialog.html | 3 + tests/unit/dialog/dialog_deprecated.html | 62 ++++++++++++++++++ tests/unit/dialog/dialog_deprecated.js | 23 +++++++ tests/unit/dialog/dialog_options.js | 20 ------ ui/jquery.ui.dialog.js | 80 +++++++++++++++++------- 7 files changed, 147 insertions(+), 43 deletions(-) create mode 100644 tests/unit/dialog/dialog_deprecated.html create mode 100644 tests/unit/dialog/dialog_deprecated.js diff --git a/build/tasks/testswarm.js b/build/tasks/testswarm.js index 0685315e698..96415aa7495 100644 --- a/build/tasks/testswarm.js +++ b/build/tasks/testswarm.js @@ -15,6 +15,7 @@ var versions = { "Core": "core/core.html", "Datepicker": "datepicker/datepicker.html", "Dialog": "dialog/dialog.html", + "Dialog_deprecated": "dialog/dialog_deprecated.html", "Draggable": "draggable/draggable.html", "Droppable": "droppable/droppable.html", "Effects": "effects/effects.html", diff --git a/tests/unit/all.html b/tests/unit/all.html index 7581a737b5e..25116846fcb 100644 --- a/tests/unit/all.html +++ b/tests/unit/all.html @@ -22,6 +22,7 @@ "core/core.html", "datepicker/datepicker.html", "dialog/dialog.html", + "dialog/dialog_deprecated.html", "draggable/draggable.html", "droppable/droppable.html", "effects/effects.html", diff --git a/tests/unit/dialog/dialog.html b/tests/unit/dialog/dialog.html index 0b5ca485626..5413e7cc1bf 100644 --- a/tests/unit/dialog/dialog.html +++ b/tests/unit/dialog/dialog.html @@ -5,6 +5,9 @@ jQuery UI Dialog Test Suite + diff --git a/tests/unit/dialog/dialog_deprecated.html b/tests/unit/dialog/dialog_deprecated.html new file mode 100644 index 00000000000..2a876ac735c --- /dev/null +++ b/tests/unit/dialog/dialog_deprecated.html @@ -0,0 +1,62 @@ + + + + + jQuery UI Dialog Test Suite + + + + + + + + + + + + + + + + + + + + + +

jQuery UI Dialog Test Suite

+

+
+

+
    +
    +
    +
    +
    +
    + Please share some personal information + + +
    +
    +

    Some more (optional) information

    + +
    +
    +
    + + diff --git a/tests/unit/dialog/dialog_deprecated.js b/tests/unit/dialog/dialog_deprecated.js new file mode 100644 index 00000000000..06052b6bf74 --- /dev/null +++ b/tests/unit/dialog/dialog_deprecated.js @@ -0,0 +1,23 @@ +module("dialog (deprecated): position opton with array"); + +if ( !$.ui.ie ) { + test("position, right bottom on window w/array", function() { + expect( 2 ); + var el = $('
    ').dialog({ position: ["right", "bottom"] }), + dialog = el.dialog('widget'), + offset = dialog.offset(); + closeEnough(offset.left, $(window).width() - dialog.outerWidth() + $(window).scrollLeft(), 1); + closeEnough(offset.top, $(window).height() - dialog.outerHeight() + $(window).scrollTop(), 1); + el.remove(); + }); +} + +test("position, offset from top left w/array", function() { + expect( 2 ); + var el = $('
    ').dialog({ position: [10, 10] }), + dialog = el.dialog('widget'), + offset = dialog.offset(); + closeEnough(offset.left, 10 + $(window).scrollLeft(), 1); + closeEnough(offset.top, 10 + $(window).scrollTop(), 1); + el.remove(); +}); diff --git a/tests/unit/dialog/dialog_options.js b/tests/unit/dialog/dialog_options.js index beb60a642e3..4e11ecf6266 100644 --- a/tests/unit/dialog/dialog_options.js +++ b/tests/unit/dialog/dialog_options.js @@ -331,16 +331,6 @@ if ( !$.ui.ie ) { el.remove(); }); - test("position, right bottom on window w/array", function() { - expect( 2 ); - var el = $('
    ').dialog({ position: ["right", "bottom"] }), - dialog = el.dialog('widget'), - offset = dialog.offset(); - closeEnough(offset.left, $(window).width() - dialog.outerWidth() + $(window).scrollLeft(), 1); - closeEnough(offset.top, $(window).height() - dialog.outerHeight() + $(window).scrollTop(), 1); - el.remove(); - }); - test("position, right bottom at right bottom via ui.position args", function() { expect( 2 ); var el = $('
    ').dialog({ @@ -359,16 +349,6 @@ if ( !$.ui.ie ) { } -test("position, offset from top left w/array", function() { - expect( 2 ); - var el = $('
    ').dialog({ position: [10, 10] }), - dialog = el.dialog('widget'), - offset = dialog.offset(); - closeEnough(offset.left, 10 + $(window).scrollLeft(), 1); - closeEnough(offset.top, 10 + $(window).scrollTop(), 1); - el.remove(); -}); - test("position, at another element", function() { expect( 4 ); var parent = $('
    ').css({ diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index 8e5ca66c1fd..781fa8fa6d1 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -211,7 +211,7 @@ $.widget("ui.dialog", { this.opener = $( this.document[ 0 ].activeElement ); this._size(); - this._position( this.options.position ); + this._position(); if ( this.options.modal ) { this.overlay = new $.ui.dialog.overlay( this ); } @@ -500,38 +500,24 @@ $.widget("ui.dialog", { } }, - _position: function( position ) { - var myAt = [], - offset = [ 0, 0 ], + _position: function() { + var position = this.options.position, + myAt = [], isVisible; if ( position ) { - // TODO we don't support 1.3.2 anymore, clean this mess up - // deep extending converts arrays to objects in jQuery <= 1.3.2 :-( - // if (typeof position == 'string' || $.isArray(position)) { - // myAt = $.isArray(position) ? position : position.split(' '); - - if ( typeof position === "string" || (typeof position === "object" && "0" in position ) ) { - myAt = position.split ? position.split( " " ) : [ position[ 0 ], position[ 1 ] ]; + if ( typeof position === "string" ) { + myAt = position.split( " " ); if ( myAt.length === 1 ) { myAt[ 1 ] = myAt[ 0 ]; } - - $.each( [ "left", "top" ], function( i, offsetPosition ) { - if ( +myAt[ i ] === myAt[ i ] ) { - offset[ i ] = myAt[ i ]; - myAt[ i ] = offsetPosition; - } - }); - position = { - my: myAt[0] + (offset[0] < 0 ? offset[0] : "+" + offset[0]) + " " + - myAt[1] + (offset[1] < 0 ? offset[1] : "+" + offset[1]), + my: myAt[0] + " " + myAt[1], at: myAt.join( " " ) }; + position = $.extend( {}, $.ui.dialog.prototype.options.position, position ); } - position = $.extend( {}, $.ui.dialog.prototype.options.position, position ); } else { position = $.ui.dialog.prototype.options.position; } @@ -610,7 +596,7 @@ $.widget("ui.dialog", { } if ( key === "position" ) { - this._position( value ); + this._position(); } if ( key === "resizable" ) { @@ -744,4 +730,52 @@ $.extend( $.ui.dialog.overlay.prototype, { } }); +// DEPRECATED +if ( $.uiBackCompat !== false ) { + // position option with array notation + // just override with old implementation + $.ui.dialog.prototype._position = function() { + var position = this.options.position, + myAt = [], + offset = [ 0, 0 ], + isVisible; + + if ( position ) { + if ( typeof position === "string" || (typeof position === "object" && "0" in position ) ) { + myAt = position.split ? position.split( " " ) : [ position[ 0 ], position[ 1 ] ]; + if ( myAt.length === 1 ) { + myAt[ 1 ] = myAt[ 0 ]; + } + + $.each( [ "left", "top" ], function( i, offsetPosition ) { + if ( +myAt[ i ] === myAt[ i ] ) { + offset[ i ] = myAt[ i ]; + myAt[ i ] = offsetPosition; + } + }); + + position = { + my: myAt[0] + (offset[0] < 0 ? offset[0] : "+" + offset[0]) + " " + + myAt[1] + (offset[1] < 0 ? offset[1] : "+" + offset[1]), + at: myAt.join( " " ) + }; + } + + position = $.extend( {}, $.ui.dialog.prototype.options.position, position ); + } else { + position = $.ui.dialog.prototype.options.position; + } + + // need to show the dialog to get the actual offset in the position plugin + isVisible = this.uiDialog.is( ":visible" ); + if ( !isVisible ) { + this.uiDialog.show(); + } + this.uiDialog.position( position ); + if ( !isVisible ) { + this.uiDialog.hide(); + } + }; +} + }( jQuery ) ); From 41c2afd66b253df2433d730631b53aef4d80f28c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Sat, 17 Nov 2012 13:07:03 +0100 Subject: [PATCH 34/43] Dialog: Refactor overlay handling into two instance methods. Remove unaddressable TODOs. --- ui/jquery.ui.dialog.js | 67 ++++++++++++++++-------------------------- 1 file changed, 26 insertions(+), 41 deletions(-) diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index 781fa8fa6d1..15e19d7cfb5 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -122,9 +122,7 @@ $.widget("ui.dialog", { var next, oldPosition = this.oldPosition; - if ( this.overlay ) { - this.overlay.destroy(); - } + this._destroyOverlay(); this.uiDialog.hide(); this.element .removeUniqueId() @@ -168,9 +166,7 @@ $.widget("ui.dialog", { this._isOpen = false; - if ( this.overlay ) { - this.overlay.destroy(); - } + this._destroyOverlay(); if ( !this.opener.filter( ":focusable" ).focus().length ) { // Hiding a focused element doesn't trigger blur in WebKit @@ -212,9 +208,7 @@ $.widget("ui.dialog", { this._size(); this._position(); - if ( this.options.modal ) { - this.overlay = new $.ui.dialog.overlay( this ); - } + this._createOverlay(); this._moveToTop( null, true ); this._show( this.uiDialog, this.options.show ); @@ -663,24 +657,13 @@ $.widget("ui.dialog", { if (this.uiDialog.is( ":data(ui-resizable)" ) ) { this.uiDialog.resizable( "option", "minHeight", this._minHeight() ); } - } -}); - -$.extend($.ui.dialog, { - // TODO move to dialog instance method - overlay: function( dialog ) { - this.$el = $.ui.dialog.overlay.create( dialog ); - } -}); + }, -// TODO get rid of instance list, at least the oldInstance stuff, and inline as dialog methods -$.extend( $.ui.dialog.overlay, { - instances: [], - // reuse old instances due to IE memory leak with alpha transparency (see #5185) - oldInstances: [], - create: function( dialog ) { - if ( this.instances.length === 0 ) { - // TODO get rid of the timeout, which should remove the need for the #4065 workaround as well + _createOverlay: function() { + if ( !this.options.modal ) { + return; + } + if ( $.ui.dialog.overlay.instances.length === 0 ) { // prevent use of anchors and inputs // we use a setTimeout in case the overlay is created from an // event that we're going to be cancelling (see #2804) @@ -697,38 +680,40 @@ $.extend( $.ui.dialog.overlay, { }, 1 ); } - var $el = ( this.oldInstances.pop() || $( "
    " ).addClass( "ui-widget-overlay ui-front" ) ); + // reuse old instances due to IE memory leak with alpha transparency (see #5185) + var $el = this.overlay = ( $.ui.dialog.overlay.oldInstances.pop() || $( "
    " ).addClass( "ui-widget-overlay ui-front" ) ); $el.appendTo( document.body ); - $el.bind( "mousedown", function( event ) { - dialog._keepFocus( event ); + this._on( $el, { + mousedown: "_keepFocus" }); - this.instances.push( $el ); - return $el; + $.ui.dialog.overlay.instances.push( $el ); }, - destroy: function( $el ) { - var indexOf = $.inArray( $el, this.instances ); + _destroyOverlay: function() { + if ( !this.options.modal ) { + return; + } + var indexOf = $.inArray( this.overlay, $.ui.dialog.overlay.instances ); if ( indexOf !== -1 ) { - this.oldInstances.push( this.instances.splice( indexOf, 1 )[ 0 ] ); + $.ui.dialog.overlay.oldInstances.push( $.ui.dialog.overlay.instances.splice( indexOf, 1 )[ 0 ] ); } - if ( this.instances.length === 0 ) { + if ( $.ui.dialog.overlay.instances.length === 0 ) { $( [ document, window ] ).unbind( ".dialog-overlay" ); } - $el.remove(); + this.overlay.remove(); } }); -$.extend( $.ui.dialog.overlay.prototype, { - destroy: function() { - $.ui.dialog.overlay.destroy( this.$el ); - } -}); +$.ui.dialog.overlay = { + instances: [], + oldInstances: [] +}; // DEPRECATED if ( $.uiBackCompat !== false ) { From 32a893128dcda19f570dbd8f07d82e6e8d434cdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Sat, 17 Nov 2012 13:14:04 +0100 Subject: [PATCH 35/43] Dialog: Improve _destroy method, detaching dialog content from wrapper instead of appending to body. --- ui/jquery.ui.dialog.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index 15e19d7cfb5..b998c2558e1 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -123,20 +123,20 @@ $.widget("ui.dialog", { oldPosition = this.oldPosition; this._destroyOverlay(); - this.uiDialog.hide(); + this.element .removeUniqueId() .removeClass( "ui-dialog-content ui-widget-content" ) .hide() - // TODO restore old position directly, instead of appending to body first - .appendTo( "body" ); + // without detaching first, the following becomes really slow + .detach(); + this.uiDialog.remove(); if ( this.originalTitle ) { this.element.attr( "title", this.originalTitle ); } - // TODO do this before removing the wrapper next = oldPosition.parent.children().eq( oldPosition.index ); // Don't try to place the dialog next to itself (#8613) if ( next.length && next[ 0 ] !== this.element[ 0 ] ) { From 5aac8f563f3ff7227fe9790c2ac8620223e3bd45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Sat, 17 Nov 2012 13:23:57 +0100 Subject: [PATCH 36/43] Dialog: Add missing unit test for aria-describedby attribute --- tests/unit/dialog/dialog_core.js | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/tests/unit/dialog/dialog_core.js b/tests/unit/dialog/dialog_core.js index 31b245a61f1..b5566255ccf 100644 --- a/tests/unit/dialog/dialog_core.js +++ b/tests/unit/dialog/dialog_core.js @@ -17,24 +17,18 @@ test("title id", function() { el.remove(); }); -// TODO test for aria-describedby -// add only when the attribute isn't anywhere yet -test("ARIA", function() { - expect(4); - - var labelledBy, - el = $('
    ').dialog(); - - equal(el.dialog('widget').attr('role'), 'dialog', 'dialog role'); - - labelledBy = el.dialog('widget').attr('aria-labelledby'); - ok(labelledBy.length > 0, 'has aria-labelledby attribute'); - equal(el.dialog('widget').find('.ui-dialog-title').attr('id'), labelledBy, - 'proper aria-labelledby attribute'); - - equal(el.dialog('widget').find('.ui-dialog-titlebar-close').attr('role'), 'button', - 'close link role'); +test( "ARIA", function() { + expect( 4 ); + + var el = $( "
    " ).dialog(), + wrapper = el.dialog( "widget" ); + equal( wrapper.attr( "role" ), "dialog", "dialog role" ); + equal( wrapper.attr( "aria-labelledby" ), wrapper.find( ".ui-dialog-title" ).attr( "id" ) ); + equal( wrapper.attr( "aria-describedby" ), el.attr( "id" ), "aria-describedby added" ); + el.remove(); + el = $( '

    descriotion

    ' ).dialog(); + strictEqual( el.dialog( "widget" ).attr( "aria-describedby" ), undefined, "no aria-describedby added, as already present in markup" ); el.remove(); }); From 73533d9bbc85ef14698668c3305359aa5522e07d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Sat, 17 Nov 2012 15:57:26 +0100 Subject: [PATCH 37/43] Dialog: Exclude deprecated units from phantomjs --- grunt.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grunt.js b/grunt.js index f43648870e3..d599496cc29 100644 --- a/grunt.js +++ b/grunt.js @@ -281,7 +281,7 @@ grunt.initConfig({ files: grunt.file.expandFiles( "tests/unit/**/*.html" ).filter(function( file ) { // disabling everything that doesn't (quite) work with PhantomJS for now // TODO except for all|index|test, try to include more as we go - return !( /(all|index|test|dialog|tabs|tooltip)\.html$/ ).test( file ); + return !( /(all|index|test|dialog|dialog_deprecated|tabs|tooltip)\.html$/ ).test( file ); }) }, lint: { From f3525afe0ec8d6599e2c54571b005b4c3d754352 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Sat, 17 Nov 2012 18:04:10 +0100 Subject: [PATCH 38/43] Dialog: Update focus-tabbable test with a timer workaround to get IE8 to pass. --- tests/unit/dialog/dialog_core.js | 52 +++++++++++++++++--------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/tests/unit/dialog/dialog_core.js b/tests/unit/dialog/dialog_core.js index b5566255ccf..404e8177027 100644 --- a/tests/unit/dialog/dialog_core.js +++ b/tests/unit/dialog/dialog_core.js @@ -48,34 +48,36 @@ test( "focus tabbable", function() { }] }; - // 1. first element inside the dialog matching [autofocus] el = $( "
    " ).dialog( options ); - equal( document.activeElement, el.find( "input" )[ 1 ] ); + equal( document.activeElement, el.find( "input" )[ 1 ], "1. first element inside the dialog matching [autofocus]" ); el.remove(); - // 2. tabbable element inside the content element - el = $( "
    " ).dialog( options ); - equal( document.activeElement, el.find( "input" )[ 0 ] ); - el.remove(); - - // 3. tabbable element inside the buttonpane - el = $( "
    text
    " ).dialog( options ); - equal( document.activeElement, el.dialog( "widget" ).find( ".ui-dialog-buttonpane button" )[ 0 ] ); - el.remove(); - - // 4. the close button - el = $( "
    text
    " ).dialog(); - equal( document.activeElement, el.dialog( "widget" ).find( ".ui-dialog-titlebar .ui-dialog-titlebar-close" )[ 0 ] ); - el.remove(); - - // 5. the dialog itself - el = $( "
    text
    " ).dialog({ - autoOpen: false - }); - el.dialog( "widget" ).find( ".ui-dialog-titlebar-close" ).hide(); - el.dialog( "open" ); - equal( document.activeElement, el.parent()[ 0 ] ); - el.remove(); + // IE8 fails to focus the input, ends up being the activeElement + // so wait for that stupid browser + stop(); + setTimeout(function() { + el = $( "
    " ).dialog( options ); + equal( document.activeElement, el.find( "input" )[ 0 ], "2. tabbable element inside the content element" ); + el.remove(); + + el = $( "
    text
    " ).dialog( options ); + equal( document.activeElement, el.dialog( "widget" ).find( ".ui-dialog-buttonpane button" )[ 0 ], "3. tabbable element inside the buttonpane" ); + el.remove(); + + el = $( "
    text
    " ).dialog(); + equal( document.activeElement, el.dialog( "widget" ).find( ".ui-dialog-titlebar .ui-dialog-titlebar-close" )[ 0 ], "4. the close button" ); + el.remove(); + + el = $( "
    text
    " ).dialog({ + autoOpen: false + }); + el.dialog( "widget" ).find( ".ui-dialog-titlebar-close" ).hide(); + el.dialog( "open" ); + equal( document.activeElement, el.parent()[ 0 ], "5. the dialog itself" ); + el.remove(); + + start(); + }, 13); }); })(jQuery); From a09f5c07f591d0ef198f1a36fab9d4b6061ecbc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Wed, 21 Nov 2012 16:33:46 +0100 Subject: [PATCH 39/43] Dialog: Follow-up to 9fe3a62d8 - also deprecate string notation for position option. --- tests/unit/dialog/dialog_deprecated.js | 32 +++++++- tests/unit/dialog/dialog_options.js | 32 -------- ui/jquery.ui.dialog.js | 101 ++++++++++--------------- 3 files changed, 72 insertions(+), 93 deletions(-) diff --git a/tests/unit/dialog/dialog_deprecated.js b/tests/unit/dialog/dialog_deprecated.js index 06052b6bf74..fcbd22faa67 100644 --- a/tests/unit/dialog/dialog_deprecated.js +++ b/tests/unit/dialog/dialog_deprecated.js @@ -1,4 +1,4 @@ -module("dialog (deprecated): position opton with array"); +module("dialog (deprecated): position option with string and array"); if ( !$.ui.ie ) { test("position, right bottom on window w/array", function() { @@ -10,6 +10,16 @@ if ( !$.ui.ie ) { closeEnough(offset.top, $(window).height() - dialog.outerHeight() + $(window).scrollTop(), 1); el.remove(); }); + + test("position, right bottom on window", function() { + expect( 2 ); + var el = $('
    ').dialog({ position: "right bottom" }), + dialog = el.dialog('widget'), + offset = dialog.offset(); + closeEnough(offset.left, $(window).width() - dialog.outerWidth() + $(window).scrollLeft(), 1); + closeEnough(offset.top, $(window).height() - dialog.outerHeight() + $(window).scrollTop(), 1); + el.remove(); + }); } test("position, offset from top left w/array", function() { @@ -21,3 +31,23 @@ test("position, offset from top left w/array", function() { closeEnough(offset.top, 10 + $(window).scrollTop(), 1); el.remove(); }); + +test("position, top on window", function() { + expect( 2 ); + var el = $('
    ').dialog({ position: "top" }), + dialog = el.dialog('widget'), + offset = dialog.offset(); + closeEnough(offset.left, Math.round($(window).width() / 2 - dialog.outerWidth() / 2) + $(window).scrollLeft(), 1); + closeEnough(offset.top, $(window).scrollTop(), 1); + el.remove(); +}); + +test("position, left on window", function() { + expect( 2 ); + var el = $('
    ').dialog({ position: "left" }), + dialog = el.dialog('widget'), + offset = dialog.offset(); + closeEnough(offset.left, 0, 1); + closeEnough(offset.top, Math.round($(window).height() / 2 - dialog.outerHeight() / 2) + $(window).scrollTop(), 1); + el.remove(); +}); diff --git a/tests/unit/dialog/dialog_options.js b/tests/unit/dialog/dialog_options.js index 4e11ecf6266..f01a1a2a896 100644 --- a/tests/unit/dialog/dialog_options.js +++ b/tests/unit/dialog/dialog_options.js @@ -298,39 +298,8 @@ test("position, default center on window", function() { el.remove(); }); -test("position, top on window", function() { - expect( 2 ); - var el = $('
    ').dialog({ position: "top" }), - dialog = el.dialog('widget'), - offset = dialog.offset(); - closeEnough(offset.left, Math.round($(window).width() / 2 - dialog.outerWidth() / 2) + $(window).scrollLeft(), 1); - closeEnough(offset.top, $(window).scrollTop(), 1); - el.remove(); -}); - -test("position, left on window", function() { - expect( 2 ); - var el = $('
    ').dialog({ position: "left" }), - dialog = el.dialog('widget'), - offset = dialog.offset(); - closeEnough(offset.left, 0, 1); - closeEnough(offset.top, Math.round($(window).height() / 2 - dialog.outerHeight() / 2) + $(window).scrollTop(), 1); - el.remove(); -}); - // todo: figure out these fails in IE7 if ( !$.ui.ie ) { - - test("position, right bottom on window", function() { - expect( 2 ); - var el = $('
    ').dialog({ position: "right bottom" }), - dialog = el.dialog('widget'), - offset = dialog.offset(); - closeEnough(offset.left, $(window).width() - dialog.outerWidth() + $(window).scrollLeft(), 1); - closeEnough(offset.top, $(window).height() - dialog.outerHeight() + $(window).scrollTop(), 1); - el.remove(); - }); - test("position, right bottom at right bottom via ui.position args", function() { expect( 2 ); var el = $('
    ').dialog({ @@ -346,7 +315,6 @@ if ( !$.ui.ie ) { closeEnough(offset.top, $(window).height() - dialog.outerHeight() + $(window).scrollTop(), 1); el.remove(); }); - } test("position, at another element", function() { diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index b998c2558e1..b9f73e954cd 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -495,33 +495,12 @@ $.widget("ui.dialog", { }, _position: function() { - var position = this.options.position, - myAt = [], - isVisible; - - if ( position ) { - if ( typeof position === "string" ) { - myAt = position.split( " " ); - if ( myAt.length === 1 ) { - myAt[ 1 ] = myAt[ 0 ]; - } - position = { - my: myAt[0] + " " + myAt[1], - at: myAt.join( " " ) - }; - position = $.extend( {}, $.ui.dialog.prototype.options.position, position ); - } - - } else { - position = $.ui.dialog.prototype.options.position; - } - // need to show the dialog to get the actual offset in the position plugin - isVisible = this.uiDialog.is( ":visible" ); + var isVisible = this.uiDialog.is( ":visible" ); if ( !isVisible ) { this.uiDialog.show(); } - this.uiDialog.position( position ); + this.uiDialog.position( this.options.position ); if ( !isVisible ) { this.uiDialog.hide(); } @@ -719,48 +698,50 @@ $.ui.dialog.overlay = { if ( $.uiBackCompat !== false ) { // position option with array notation // just override with old implementation - $.ui.dialog.prototype._position = function() { - var position = this.options.position, - myAt = [], - offset = [ 0, 0 ], - isVisible; - - if ( position ) { - if ( typeof position === "string" || (typeof position === "object" && "0" in position ) ) { - myAt = position.split ? position.split( " " ) : [ position[ 0 ], position[ 1 ] ]; - if ( myAt.length === 1 ) { - myAt[ 1 ] = myAt[ 0 ]; - } - - $.each( [ "left", "top" ], function( i, offsetPosition ) { - if ( +myAt[ i ] === myAt[ i ] ) { - offset[ i ] = myAt[ i ]; - myAt[ i ] = offsetPosition; + $.widget( "ui.dialog", $.ui.dialog, { + _position: function() { + var position = this.options.position, + myAt = [], + offset = [ 0, 0 ], + isVisible; + + if ( position ) { + if ( typeof position === "string" || (typeof position === "object" && "0" in position ) ) { + myAt = position.split ? position.split( " " ) : [ position[ 0 ], position[ 1 ] ]; + if ( myAt.length === 1 ) { + myAt[ 1 ] = myAt[ 0 ]; } - }); - position = { - my: myAt[0] + (offset[0] < 0 ? offset[0] : "+" + offset[0]) + " " + - myAt[1] + (offset[1] < 0 ? offset[1] : "+" + offset[1]), - at: myAt.join( " " ) - }; - } + $.each( [ "left", "top" ], function( i, offsetPosition ) { + if ( +myAt[ i ] === myAt[ i ] ) { + offset[ i ] = myAt[ i ]; + myAt[ i ] = offsetPosition; + } + }); - position = $.extend( {}, $.ui.dialog.prototype.options.position, position ); - } else { - position = $.ui.dialog.prototype.options.position; - } + position = { + my: myAt[0] + (offset[0] < 0 ? offset[0] : "+" + offset[0]) + " " + + myAt[1] + (offset[1] < 0 ? offset[1] : "+" + offset[1]), + at: myAt.join( " " ) + }; + } - // need to show the dialog to get the actual offset in the position plugin - isVisible = this.uiDialog.is( ":visible" ); - if ( !isVisible ) { - this.uiDialog.show(); - } - this.uiDialog.position( position ); - if ( !isVisible ) { - this.uiDialog.hide(); + position = $.extend( {}, $.ui.dialog.prototype.options.position, position ); + } else { + position = $.ui.dialog.prototype.options.position; + } + + // need to show the dialog to get the actual offset in the position plugin + isVisible = this.uiDialog.is( ":visible" ); + if ( !isVisible ) { + this.uiDialog.show(); + } + this.uiDialog.position( position ); + if ( !isVisible ) { + this.uiDialog.hide(); + } } - }; + }); } }( jQuery ) ); From ec1f1bde76c555e7643899c5bbbb200353f780ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Thu, 22 Nov 2012 10:39:45 +0100 Subject: [PATCH 40/43] Dialog: Follow-up to c77ca67 - exclude button options from properties to create the button. --- ui/jquery.ui.dialog.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index b9f73e954cd..ea5226ac5ff 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -376,7 +376,7 @@ $.widget("ui.dialog", { if ( !$.isEmptyObject( buttons ) ) { $.each( buttons, function( name, props ) { - var click; + var click, buttonOptions; props = $.isFunction( props ) ? { click: props, text: name } : props; @@ -387,11 +387,14 @@ $.widget("ui.dialog", { props.click = function() { click.apply( that.element[0], arguments ); }; + buttonOptions = { + icons: props.icons, + text: props.showText + }; + delete props.icons; + delete props.showText; $( "", props ) - .button({ - icons: props.icons, - text: props.showText - }) + .button( buttonOptions ) .appendTo( that.uiButtonSet ); }); this.uiDialog.addClass( "ui-dialog-buttons" ); From d179cbaf3233ace0bc542e836c5c46e4129a9e0a Mon Sep 17 00:00:00 2001 From: Kris Borchers Date: Sat, 24 Nov 2012 23:18:51 -0600 Subject: [PATCH 41/43] Dialog: Update position when size is changed. Fixes #8789 - Dialog does not close for first click on chrome. --- tests/visual/dialog/complex-dialogs.html | 3 +-- ui/jquery.ui.dialog.js | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/visual/dialog/complex-dialogs.html b/tests/visual/dialog/complex-dialogs.html index 5c2e1d8a12c..5cd9271aef2 100644 --- a/tests/visual/dialog/complex-dialogs.html +++ b/tests/visual/dialog/complex-dialogs.html @@ -24,7 +24,6 @@ $(function() { var dialog = $( "#dialog" ).dialog({ modal: true, - height: 350, width: 500, buttons: [ { @@ -45,7 +44,7 @@ showText: false } ] - }), + }).dialog("option", "height", 600), datepickerDialog = $( "#dialog-datepicker" ).dialog({ autoOpen: false, diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index ea5226ac5ff..bcfc6f07c79 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -527,6 +527,7 @@ $.widget("ui.dialog", { if ( resize ) { this._size(); + this._position(); } if ( this.uiDialog.is( ":data(ui-resizable)" ) ) { this.uiDialog.resizable( "option", resizableOptions ); From 60486ac632a0a1bbbb0c7449fe17bccfae11af80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Mon, 26 Nov 2012 10:08:34 +0100 Subject: [PATCH 42/43] Dialog: Don't focus dialog when mousedown is on close button. Fixes #8838 - Dialog: Close icon does not work in dialog larger than the window in IE. --- ui/jquery.ui.dialog.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index bcfc6f07c79..5eb3e2aca82 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -321,9 +321,14 @@ $.widget("ui.dialog", { .addClass( "ui-dialog-titlebar ui-widget-header ui-corner-all ui-helper-clearfix" ) .prependTo( this.uiDialog ); this._on( this.uiDialogTitlebar, { - mousedown: function() { - // Dialog isn't getting focus when dragging (#8063) - this.uiDialog.focus(); + mousedown: function( event ) { + // Don't prevent click on close button (#8838) + // Focusing a dialog that is partially scrolled out of view + // causes the browser to scroll it into view, preventing the click event + if ( !$( event.target ).closest( ".ui-dialog-titlebar-close" ) ) { + // Dialog isn't getting focus when dragging (#8063) + this.uiDialog.focus(); + } } }); From 7e9060c109b928769a664dbcc2c17bd21231b6f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Zaefferer?= Date: Mon, 26 Nov 2012 10:14:36 +0100 Subject: [PATCH 43/43] Dialog: Extract setting the title into a _title method, use .text() to prevent XSS. Fixes #6016 - Dialog: Title XSS Vulnerability. --- ui/jquery.ui.dialog.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/ui/jquery.ui.dialog.js b/ui/jquery.ui.dialog.js index 5eb3e2aca82..808d31d5b2b 100644 --- a/ui/jquery.ui.dialog.js +++ b/ui/jquery.ui.dialog.js @@ -352,14 +352,21 @@ $.widget("ui.dialog", { uiDialogTitle = $( "" ) .uniqueId() .addClass( "ui-dialog-title" ) - .html( this.options.title || " " ) .prependTo( this.uiDialogTitlebar ); + this._title( uiDialogTitle ); this.uiDialog.attr({ "aria-labelledby": uiDialogTitle.attr( "id" ) }); }, + _title: function( title ) { + if ( !this.options.title ) { + title.html( " " ); + } + title.text( this.options.title ); + }, + _createButtonPane: function() { var uiDialogButtonPane = ( this.uiDialogButtonPane = $( "
    " ) ) .addClass( "ui-dialog-buttonpane ui-widget-content ui-helper-clearfix" ); @@ -600,9 +607,7 @@ $.widget("ui.dialog", { } if ( key === "title" ) { - // convert whatever was passed in to a string, for html() to not throw up - $( ".ui-dialog-title", this.uiDialogTitlebar ) - .html( "" + ( value || " " ) ); + this._title( this.uiDialogTitlebar.find( ".ui-dialog-title" ) ); } },