Toolbar: Fixed toolbar show/hide must not interfere with each other

Fixes gh-7906
Fixes gh-5514
Closes gh-7902

js/index.php

@@ -77,6 +77,7 @@
 	'links.js',
 	'widgets/toolbar.js',
 	'widgets/fixedToolbar.js',
+	'widgets/fixedToolbar.backcompat.js',
 	'widgets/fixedToolbar.workarounds.js',
 	'widgets/panel.js',
 	'widgets/popup.js',

js/jquery.mobile.js

@@ -42,6 +42,7 @@ define([
 	"./links",
 	"./widgets/toolbar",
 	"./widgets/fixedToolbar",
+	"./widgets/fixedToolbar.backcompat",
 	"./widgets/fixedToolbar.workarounds",
 	"./widgets/popup",
 	"./widgets/popup.arrow",

js/widgets/fixedToolbar.backcompat.js

@@ -0,0 +1,78 @@
+//>>excludeStart("jqmBuildExclude", pragmas.jqmBuildExclude);
+//>>description: Backcompat features fo fixed toolbars
+//>>label: Toolbars: Fixed
+//>>group: Widgets
+
+define( [ "jquery", "./fixedToolbar" ], function( jQuery ) {
+
+//>>excludeEnd("jqmBuildExclude");
+
+( function( $, undefined ) {
+
+if ( $.mobileBackcompat !== false ) {
+
+	$.widget( "mobile.toolbar", $.mobile.toolbar, {
+
+		options: {
+			hideDuringFocus: "input, textarea, select"
+		},
+
+		_hideDuringFocusData: {
+			delayShow: 0,
+			delayHide: 0,
+			isVisible: true
+		},
+
+		_handlePageFocusinFocusout: function( event ) {
+			var data = this._hideDuringFocusData;
+
+			// This hides the toolbars on a keyboard pop to give more screen room and prevent
+			// ios bug which positions fixed toolbars in the middle of the screen on pop if the
+			// input is near the top or bottom of the screen addresses issues #4410 Footer
+			// navbar moves up when clicking on a textbox in an Android environment and issue
+			// #4113 Header and footer change their position after keyboard popup - iOS and
+			// issue #4410 Footer navbar moves up when clicking on a textbox in an Android
+			// environment
+			if ( this.options.hideDuringFocus && screen.width < 1025 &&
+					$( event.target ).is( this.options.hideDuringFocus ) &&
+					!$( event.target )
+						.closest( ".ui-header-fixed, .ui-footer-fixed" ).length ) {
+
+				// Fix for issue #4724 Moving through form in Mobile Safari with "Next" and
+				// "Previous" system controls causes fixed position, tap-toggle false Header to
+				// reveal itself isVisible instead of self._visible because the focusin and
+				// focusout events fire twice at the same time Also use a delay for hiding the
+				// toolbars because on Android native browser focusin is direclty followed by a
+				// focusout when a native selects opens and the other way around when it closes.
+				if ( event.type === "focusout" && !data.isVisible ) {
+					data.isVisible = true;
+
+					// Wait for the stack to unwind and see if we have jumped to another input
+					clearTimeout( data.delayHide );
+					data.delayShow = this._delay( "show", 0 );
+				} else if ( event.type === "focusin" && !!data.isVisible ) {
+
+					// If we have jumped to another input clear the time out to cancel the show
+					clearTimeout( data.delayShow );
+						data.isVisible = false;
+						data.delayHide = this._delay( "hide", 0 );
+				}
+			}
+		},
+
+		_attachToggleHandlersToPage: function( page ) {
+			this._on( page, {
+				focusin: "_handlePageFocusinFocusout",
+				focusout: "_handlePageFocusinFocusout"
+			} );
+			return this._superApply( arguments );
+		}
+
+	} );
+
+}
+
+} )( jQuery );
+//>>excludeStart("jqmBuildExclude", pragmas.jqmBuildExclude);
+});
+//>>excludeEnd("jqmBuildExclude");

js/widgets/fixedToolbar.js

@@ -18,7 +18,6 @@ define( [ "jquery", "../widget", "../core", "../animationComplete", "../navigati
 			fullscreen: false,
 			tapToggle: true,
 			tapToggleBlacklist: "a, button, input, select, textarea, .ui-header-fixed, .ui-footer-fixed, .ui-flipswitch, .ui-popup, .ui-panel, .ui-panel-dismiss-open",
-			hideDuringFocus: "input, textarea, select",
 			updatePagePadding: true,
 			trackPersistentToolbars: true,
 
@@ -189,12 +188,17 @@ define( [ "jquery", "../widget", "../core", "../animationComplete", "../navigati
 				$el = this.element;
 
 			if ( this._useTransition( notransition ) ) {
+				this._animationInProgress = "show";
 				$el
 					.removeClass( "out " + hideClass )
 					.addClass( "in" )
-					.animationComplete(function () {
-						$el.removeClass( "in" );
-					});
+					.animationComplete( $.proxy( function () {
+						if ( this._animationInProgress === "show" ) {
+							this._animationInProgress = false;
+
+							$el.removeClass( "in" );
+						}
+					}, this ) );
 			}
 			else {
 				$el.removeClass( hideClass );
@@ -209,12 +213,17 @@ define( [ "jquery", "../widget", "../core", "../animationComplete", "../navigati
 				outclass = "out" + ( this.options.transition === "slide" ? " reverse" : "" );
 
 			if ( this._useTransition( notransition ) ) {
+				this._animationInProgress = "hide";
 				$el
 					.addClass( outclass )
 					.removeClass( "in" )
-					.animationComplete(function() {
-						$el.addClass( hideClass ).removeClass( outclass );
-					});
+					.animationComplete( $.proxy( function() {
+						if ( this._animationInProgress === "hide" ) {
+							this._animationInProgress = false;
+
+							$el.addClass( hideClass ).removeClass( outclass );
+						}
+					}, this ) );
 			}
 			else {
 				$el.addClass( hideClass ).removeClass( outclass );
@@ -227,47 +236,19 @@ define( [ "jquery", "../widget", "../core", "../animationComplete", "../navigati
 		},
 
 		_bindToggleHandlers: function() {
+			this._attachToggleHandlersToPage( ( !!this.page ) ? this.page: $( ".ui-page" ) );
+		},
+
+		_attachToggleHandlersToPage: function( page ) {
 			var self = this,
-				o = self.options,
-				delayShow, delayHide,
-				isVisible = true,
-				page = ( !!this.page )? this.page: $(".ui-page");
+				o = self.options;
 
 			// tap toggle
 			page
 				.bind( "vclick", function( e ) {
 					if ( o.tapToggle && !$( e.target ).closest( o.tapToggleBlacklist ).length ) {
 						self.toggle();
 					}
-				})
-				.bind( "focusin focusout", function( e ) {
-					//this hides the toolbars on a keyboard pop to give more screen room and prevent ios bug which
-					//positions fixed toolbars in the middle of the screen on pop if the input is near the top or
-					//bottom of the screen addresses issues #4410 Footer navbar moves up when clicking on a textbox in an Android environment
-					//and issue #4113 Header and footer change their position after keyboard popup - iOS
-					//and issue #4410 Footer navbar moves up when clicking on a textbox in an Android environment
-					if ( screen.width < 1025 && $( e.target ).is( o.hideDuringFocus ) && !$( e.target ).closest( ".ui-header-fixed, .ui-footer-fixed" ).length ) {
-						//Fix for issue #4724 Moving through form in Mobile Safari with "Next" and "Previous" system
-						//controls causes fixed position, tap-toggle false Header to reveal itself
-						// isVisible instead of self._visible because the focusin and focusout events fire twice at the same time
-						// Also use a delay for hiding the toolbars because on Android native browser focusin is direclty followed
-						// by a focusout when a native selects opens and the other way around when it closes.
-						if ( e.type === "focusout" && !isVisible ) {
-							isVisible = true;
-							//wait for the stack to unwind and see if we have jumped to another input
-							clearTimeout( delayHide );
-							delayShow = setTimeout( function() {
-								self.show();
-							}, 0 );
-						} else if ( e.type === "focusin" && !!isVisible ) {
-							//if we have jumped to another input clear the time out to cancel the show.
-							clearTimeout( delayShow );
-							isVisible = false;
-							delayHide = setTimeout( function() {
-								self.hide();
-							}, 0 );
-						}
-					}
 				});
 		},
 

tests/integration/fixed-toolbar/fixedToolbar.js

@@ -367,12 +367,150 @@
 
 		        ok( $.testHelper.domEqual( destroyed, unEnhanced ),
 		        	"unEnhanced equals destroyed" );
-		        start();
-			},
-			function() {
 				$( ":mobile-pagecontainer" ).pagecontainer( "change", "#default" );
-			}
+			},
+			start
 		]);
 	});
 
+	// It is insufficient to check the final assortment of classes when ascertaining that the stale
+	// .animationComplete() handler does not do anything, because in the show/hide case the
+	// final assortment of classes happens to be correct. Thus, we must intercept calls to
+	// .addClass() and .removeClass() and make sure that only calls associated with non-stale
+	// .animationComplete() handlers take place.
+	//
+	// Create a scope for holding variables for this module
+	( function() {
+
+		var callSequence, recordCalls, toolbar, testPageClone,
+			originalAddClass = $.fn.addClass,
+			originalRemoveClass = $.fn.removeClass,
+			testPage = $( "#stale-animation-test-page" ).remove();
+
+		module( "stale animation is ignored", {
+			setup: function() {
+				recordCalls = false;
+				callSequence = [];
+				testPageClone = testPage.clone();
+				toolbar = testPageClone.appendTo( "body" ).children( "#stale-animation-test" );
+
+				scrollUp();
+
+				$.fn.addClass = function() {
+					if ( this.length && this[ 0 ] === toolbar[ 0 ] && recordCalls ) {
+						callSequence.push({
+							addClass: Array.prototype.slice.call( arguments )
+						});
+					}
+					return originalAddClass.apply( this, arguments );
+				};
+				$.fn.removeClass = function() {
+					if ( this.length && this[ 0 ] === toolbar[ 0 ] && recordCalls ) {
+						callSequence.push({
+							removeClass: Array.prototype.slice.call( arguments )
+						});
+					}
+					return originalRemoveClass.apply( this, arguments );
+				};
+			},
+			teardown: function() {
+				testPageClone.remove();
+				$.fn.addClass = originalAddClass;
+				$.fn.removeClass = originalRemoveClass;
+				scrollUp();
+			}
+		});
+
+		asyncTest( "hide() followed by show(): stale animationComplete() handler is ignored",
+			function() {
+				var expectedCallSequence = [
+
+					// These are called synchronously from hide
+					{ addClass: [ "out reverse" ] },
+					{ removeClass: [ "in" ] },
+
+					// These are called synchronously from show
+					{ removeClass: [ "out ui-fixed-hidden" ] },
+					{ addClass: [ "in" ] },
+
+					// This is called asynchronously from show()'s animationComplete handler
+					{ removeClass: [ "in" ] }
+				];
+
+				expect( 1 );
+
+				$.testHelper.pageSequence([
+					function() {
+						$( ":mobile-pagecontainer" )
+							.pagecontainer( "change", "#stale-animation-test-page" );
+					},
+					function() {
+						scrollDown();
+						recordCalls = true;
+						toolbar.toolbar( "hide" );
+						toolbar.toolbar( "show" );
+
+						// Give the animations some time
+						setTimeout( function() {
+							recordCalls = false;
+							deepEqual( callSequence, expectedCallSequence,
+								"Calls to addClass() and removeClass() made by stale " +
+									"animationComplete() handler are not present" );
+							console.log( "hide/show: " + toolbar.attr( "class" ) );
+
+							// Conclude test after having gone back to the main page
+							$.testHelper.pageSequence([ function() { $.mobile.back(); }, start ]);
+						}, 2000 );
+					}
+				]);
+			});
+
+		asyncTest( "show() followed by hide(): stale animationComplete() handler is ignored",
+			function() {
+				var expectedCallSequence = [
+
+					// These are called synchronously from show
+					{ removeClass: [ "out ui-fixed-hidden" ] },
+					{ addClass: [ "in" ] },
+
+					// These are called synchronously from hide
+					{ addClass: [ "out reverse" ] },
+					{ removeClass: [ "in" ] },
+
+					// These are called asynchronously from hide()'s animationComplete handler
+					{ addClass: [ "ui-fixed-hidden" ] },
+					{ removeClass: [ "out reverse" ] },
+				];
+
+				expect( 1 );
+
+				$.testHelper.pageSequence([
+					function() {
+						$( ":mobile-pagecontainer" )
+							.pagecontainer( "change", "#stale-animation-test-page" );
+					},
+					function() {
+						toolbar.toolbar( "hide" );
+
+						scrollDown();
+						recordCalls = true;
+						toolbar.toolbar( "show" );
+						toolbar.toolbar( "hide" );
+
+						// Give the animations some time
+						setTimeout( function() {
+							recordCalls = false;
+							deepEqual( callSequence, expectedCallSequence,
+								"Calls to addClass() and removeClass() made by stale " +
+									"animationComplete() handler are not present" );
+							console.log( "show/hide: " + toolbar.attr( "class" ) );
+
+							// Conclude test after having gone back to the main page
+							$.testHelper.pageSequence([ function() { $.mobile.back(); }, start ]);
+						}, 2000 );
+					}
+				]);
+			});
+
+	})();
 })(jQuery);

tests/integration/fixed-toolbar/index.html

@@ -110,5 +110,12 @@ <h1>foo</h1>
             <h1>title</h1>
         </div>
 	</div>
+	<div data-nstest-role="page" id="stale-animation-test-page">
+		<div id="stale-animation-test" data-nstest-position="fixed" data-nstest-role="header">
+			<h1>Toolbar</h1>
+		</div>
+		<div class="ui-content">
+		</div>
+	</div>
 </body>
 </html>