Skip to main content

Search and Top Navigation

#8579 closed bug (duplicate)

Opened September 14, 2012 11:09AM UTC

Closed October 23, 2012 08:18PM UTC

_normalizeArguments not working properly when ({effect: "someeffect"}, callback)

Reported by: sparkybg Owned by:
Priority: minor Milestone: 1.10.0
Component: ui.effects.core Version: 1.9.0-rc.1
Keywords: Cc:
Blocked by: Blocking:
Description

Summary:

If "hide" effect for a dialog is defined, "close" event is not invoked when dialog closes.

How to reproduce:

Here is it on jsbin:

http://jsbin.com/ocesik/1/edit

$("<div></div>).dialog({
   modal: true,
   autoOpen: true,
   hide: {effect: "fade", duration: 150}, //if hide effect is defined, "close" event is not triggered
   close: function () {alert("dialog:close")}
});

Possible cause:

This results in calling of:

_normalizeArgumets({effect: "fade", duration: 150}, function () {alert("dialog:close")});

but the callback is lost after the first "if" in the _normalizeArguments function:

// here "options" parameter is "function () {alert("dialog:close")}"
if ( $.isPlainObject( effect ) ) {
   options = effect; //here the callback is lost!!!
   effect = effect.effect;
}
// here "options" is the same as "effect" parameter: "{effect: "fade", duration: 150"}, so, the callback is lost and not invoked

Possible fix:

First "if" in the function should be replaced by:

if ( $.isPlainObject( effect ) ) {
   callback = options; //the callback is not lost anymore
   options = effect;
   effect = effect.effect;
}
Attachments (0)
Change History (5)

Changed October 11, 2012 02:42PM UTC by scottgonzalez comment:1

milestone: 1.9.01.10.0

Changed October 18, 2012 04:47AM UTC by acouch comment:2

owner: → acouch
status: newassigned

Changed October 18, 2012 05:58AM UTC by acouch comment:3

Indeed, the issue comes about at line 250 (at 4d5197) of jquery.ui.dialog, which makes a call to an undocumented version of .hide().

We could put an explicit check there for an object, but it does seem like a reasonable method signature.

this.uiDialog.hide( this.options.hide, function() {
        that._trigger( "close", event );
});

Changed October 18, 2012 01:11PM UTC by acouch comment:4

owner: acouch
status: assignedopen

I've developed two possible fixes, neither of them particularly clean.

As you suggested, we could update the effects show/hide methods to allow these parameters. However, this behavior is currently unspecified, so it that solution may have wider ramifications. Also, it introduces an interesting synchronization question. If both callbacks are provided, which should be executed first?

https://github.com/couchand/jquery-ui/commit/161b2bc7089a47210a0c94de381fea29fec6e709

We could also explicitly check for the object in the dialog's close method. This way we can explicitly choose, in context, which handler to call first. In my opinion, the complete callback in the hide options should be called before firing the close event.

https://github.com/couchand/jquery-ui/commit/e0345032438696bc70ee0ff3344f6e298882d529

Changed October 23, 2012 08:18PM UTC by scottgonzalez comment:5

resolution: → duplicate
status: openclosed

Duplicate of #8684.Also see #8670