Skip to content

Commit

Permalink
Autocomplete: Fall back to .ui-front searching for empty jQuery objects
Browse files Browse the repository at this point in the history 
Fixes #9755
  • Loading branch information
@scottgonzalez
scottgonzalez committed Jan 20, 2014
1 parent 0bb807b commit 2ef1b16
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 12 deletions.
37 changes: 26 additions & 11 deletions tests/unit/autocomplete/autocomplete_options.js
View file
Expand Up @@ -5,13 +5,18 @@ module( "autocomplete: options" );
var data = [ "c++", "java", "php", "coldfusion", "javascript", "asp", "ruby", var data = [ "c++", "java", "php", "coldfusion", "javascript", "asp", "ruby",
"python", "c", "scala", "groovy", "haskell", "perl" ]; "python", "c", "scala", "groovy", "haskell", "perl" ];


test( "appendTo", function() { test( "appendTo: null", function() {
expect( 8 ); expect( 1 );
var detached = $( "<div>" ), var element = $( "#autocomplete" ).autocomplete();
element = $( "#autocomplete" ).autocomplete();
equal( element.autocomplete( "widget" ).parent()[ 0 ], document.body, equal( element.autocomplete( "widget" ).parent()[ 0 ], document.body,
"defaults to body" ); "defaults to body" );
element.autocomplete( "destroy" ); element.autocomplete( "destroy" );
});

test( "appendTo: explicit", function() {
expect( 6 );
var detached = $( "<div>" ),
element = $( "#autocomplete" );


element.autocomplete({ element.autocomplete({
appendTo: ".autocomplete-wrap" appendTo: ".autocomplete-wrap"
Expand All @@ -22,13 +27,6 @@ test( "appendTo", function() {
"only appends to one element" ); "only appends to one element" );
element.autocomplete( "destroy" ); element.autocomplete( "destroy" );


$( "#autocomplete-wrap2" ).addClass( "ui-front" );
element.autocomplete();
equal( element.autocomplete( "widget" ).parent()[ 0 ],
$( "#autocomplete-wrap2" )[ 0 ], "null, inside .ui-front" );
element.autocomplete( "destroy" );
$( "#autocomlete-wrap2" ).removeClass( "ui-front" );

element.autocomplete().autocomplete( "option", "appendTo", "#autocomplete-wrap1" ); element.autocomplete().autocomplete( "option", "appendTo", "#autocomplete-wrap1" );
equal( element.autocomplete( "widget" ).parent()[ 0 ], equal( element.autocomplete( "widget" ).parent()[ 0 ],
$( "#autocomplete-wrap1" )[ 0 ], "modified after init" ); $( "#autocomplete-wrap1" )[ 0 ], "modified after init" );
Expand All @@ -54,6 +52,23 @@ test( "appendTo", function() {
element.autocomplete( "destroy" ); element.autocomplete( "destroy" );
}); });


test( "appendTo: ui-front", function() {
expect( 2 );
var element = $( "#autocomplete" );

$( "#autocomplete-wrap2" ).addClass( "ui-front" );
element.autocomplete();
equal( element.autocomplete( "widget" ).parent()[ 0 ],
$( "#autocomplete-wrap2" )[ 0 ], "null, inside .ui-front" );
element.autocomplete( "destroy" );

element.autocomplete({
appendTo: $()
});
equal( element.autocomplete( "widget" ).parent()[ 0 ],
$( "#autocomplete-wrap2" )[ 0 ], "null, inside .ui-front" );
});

function autoFocusTest( afValue, focusedLength ) { function autoFocusTest( afValue, focusedLength ) {
var element = $( "#autocomplete" ).autocomplete({ var element = $( "#autocomplete" ).autocomplete({
autoFocus: afValue, autoFocus: afValue,
Expand Down
2 changes: 1 addition & 1 deletion ui/jquery.ui.autocomplete.js
View file
Expand Up @@ -339,7 +339,7 @@ $.widget( "ui.autocomplete", {
this.document.find( element ).eq( 0 ); this.document.find( element ).eq( 0 );
} }


if ( !element ) { if ( !element || !element[ 0 ] ) {
element = this.element.closest( ".ui-front" ); element = this.element.closest( ".ui-front" );
} }


Expand Down

5 comments on commit 2ef1b16

@MoonScript
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@scottgonzalez thanks for the quick fix! I noticed a similar thing in the dialog widget:
https://github.com/jquery/jquery-ui/blob/master/ui/jquery.ui.dialog.js#L133

And there may be other widgets that use the same appendTo option, too.

@scottgonzalez
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dialog doesn't use the .ui-front walking. You can only be explicit.

@MoonScript
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but you could end up with a case where the dialog wouldn't be appended to anything if an empty jQuery collection was passed in.

@scottgonzalez
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't tend to write defensive code like this. The only reason to use dialog's appendTo option is because of forms or the like. If you pass an invalid object, the page is going to be broken anyway.

@MoonScript
Copy link

@MoonScript MoonScript commented on 2ef1b16 Jan 21, 2014 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.