Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Popup: Fixes closing the popup unexpectedly by race conditions. #668

Closed
wants to merge 4 commits into from
Closed

Popup: Fixes closing the popup unexpectedly by race conditions. #668

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c6cce9c
Popup: Added _closeTry and _closeVet methods. Fixes closing the popup…
rxaviers May 26, 2012
96c6b94
Popup: Binds document mousedown instead of click. The click event is …
rxaviers May 26, 2012
aa135de
Popup: Manually binding document instead of using _bind. Because on d…
rxaviers May 26, 2012
a3cd4fd
Popup: Lowered the closing delay from 150ms to 50ms. Makes response f…
rxaviers May 26, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 38 additions & 16 deletions ui/jquery.ui.popup.js
View file
Open in desktop
Expand Up @@ -89,7 +89,7 @@ $.widget( "ui.popup", {
case $.ui.keyCode.UP:
// prevent scrolling
event.preventDefault();
clearTimeout( this.closeTimer );
this._closeVet();
this._delay(function() {
this.open( event );
this.focusPopup( event );
Expand All @@ -114,7 +114,7 @@ $.widget( "ui.popup", {
return;
}
this.open( event );
clearTimeout( this.closeTimer );
this._closeVet();
this._delay( function() {
if ( !noFocus ) {
this.focusPopup();
Expand Down Expand Up @@ -164,17 +164,10 @@ $.widget( "ui.popup", {

this._bind({
focusout: function( event ) {
// use a timer to allow click to clear it and letting that
// handle the closing instead of opening again
this.closeTimer = this._delay( function() {
this.close( event );
}, 150);
this._closeTry( event );
},
focusin: function( event ) {
clearTimeout( this.closeTimer );
},
mouseup: function( event ) {
clearTimeout( this.closeTimer );
this._closeVet();
}
});

Expand All @@ -187,22 +180,51 @@ $.widget( "ui.popup", {
}
});

this._bind( this.document, {
click: function( event ) {
if ( this.isOpen && !$( event.target ).closest( this.element.add( this.options.trigger ) ).length ) {
this.close( event );
this.outsideHandler = $.proxy ( function ( event ) {
if ( this.isOpen ) {
if ( $( event.target ).closest( this.element.add( this.options.trigger ) ).length ) {
this._closeVet();
}
else {
this._closeTry( event );
}
}
});
}, this );
$( this.document ).mousedown( this.outsideHandler );
},

// Unless vetted, close. Use a timer to allow veto in a timed window.
// Note that a focusout followed by a focusin sometimes trigger events in reverse order, but should not close the popup nevertheless. Or a click-in-the-popup followed by a focusout should also avoid closing it unexpectedly.
_closeTry: function( event ) {
if ( this.closeVeto || this.closeTimer ) {
return;
}
this.closeTimer = this._delay( function() {
this.close( event );
this.closeTimer = null;
}, 50 );
},

// Vets any close attempt in a timed window.
_closeVet: function() {
this.closeVeto = true;
this._delay( function() {
this.closeVeto = false;
}, 50 );
if ( this.closeTimer ) {
clearTimeout( this.closeTimer );
this.closeTimer = null;
}
},

_destroy: function() {
this.element
.show()
.removeClass( "ui-popup" )
.removeAttr( "aria-hidden" )
.removeAttr( "aria-expanded" )
.unbind( "keypress.ui-popup");
$( this.document ).unbind( "mousedown", this.outsideHandler );

this.options.trigger
.removeAttr( "aria-haspopup" )
Expand Down