menubar: Repair TAB order bugs

I believe this fixes tab order.

Regrettably I cannot get the unit test (included
in this commit) which verifies the function of TAB
by use of `simulate` functioning.

The test under investigation is:

"TAB order should be sane mirroring dialog's test "

in `menubar_events.js`.

I'm including it in the PR in hopes that
additional review will present the flaw in my
implementation.  I've tried mirroring the use in
dialog's test suite with no success.

Curiously my use of `simulate` _seems_ right
insofar as my test using it to trigger RIGHT and
LEFT cursor key events _is_ behaving as expected.

Author: Steven G. Harms

tests/unit/menubar/menubar_core.js

@@ -19,4 +19,18 @@ test( "accessibility", function () {
 	ok( !element.attr( "aria-activedescendant" ), "aria-activedescendant not set" );
 });
 
+test( "Cursor keys should move the focus", function() {
+	expect( 3 );
+
+	var element = $( "#bar1" ).menubar(),
+		firstMenuItem = $( "#bar1 .ui-menubar-item .ui-button:first" );
+
+	firstMenuItem[ 0 ].focus();
+	equal( document.activeElement, firstMenuItem[0], "Focus set on first menuItem" );
+	$( firstMenuItem ).simulate( "keydown", { keyCode: $.ui.keyCode.RIGHT } );
+	ok( !firstMenuItem.hasClass( "ui-state-focus" ), "RIGHT should move focus off of focused item" );
+	$( document.activeElement ).simulate( "keydown", { keyCode: $.ui.keyCode.LEFT } );
+	equal( document.activeElement, firstMenuItem[0], "LEFT should return focus first menuItem" );
+} );
+
 })( jQuery );

tests/unit/menubar/menubar_events.js

@@ -50,13 +50,70 @@ test ( "_findNextFocusableTarget should find one and only one item", function()
 	expect(2);
 
 	var element = $("#bar1").menubar(),
-		menubarWidget = element.data("ui-menubar"),
-		firstMenuItem = $("#bar1 .ui-menubar-item").eq(0),
-		expectedFocusableTarget = $("#bar1 .ui-menubar-item .ui-widget").eq(0),
+		menubarWidget = element.data( "ui-menubar" ),
+		firstMenuItem = $( "#bar1 .ui-menubar-item" ).eq( 0 ),
+		expectedFocusableTarget = $("#bar1 .ui-menubar-item .ui-widget").eq( 0 ),
 		result = menubarWidget._findNextFocusableTarget( firstMenuItem );
 
   equal( expectedFocusableTarget[0], result[0], "_findNextFocusableTarget should return the focusable element underneath the menuItem" );
 	equal( 1, result.length, "One and only one item should be returned." );
 });
 
+asyncTest( "TAB order should be sane mirroring dialog's test", function() {
+	expect( 3 );
+
+	var element = $( "#bar1" ).menubar(),
+		firstMenuItem = $( "#bar1 .ui-menubar-item .ui-button:first" );
+
+	function checkTab() {
+		setTimeout( start );
+		ok( !firstMenuItem.hasClass( "ui-state-focus" ), "The manually focused item should no longer have focus after TAB" );
+		//setTimeout( start );
+	}
+
+	firstMenuItem[ 0 ].focus();
+	ok( $( firstMenuItem ).hasClass( "ui-state-focus" ), "Should have focus class" );
+
+	setTimeout(function() {
+		equal( document.activeElement, firstMenuItem[0], "Focus set on first menuItem" );
+		$( document.activeElement ).simulate( "keydown", { keyCode: $.ui.keyCode.TAB } );
+		setTimeout( checkTab );
+	})
+
+} );
+
+asyncTest( "TAB order should be sane", function() {
+	expect( 3 );
+
+
+	var element = $( "#bar1" ).menubar(),
+		debugDelay = 0,
+		firstMenuItem = $( "#bar1 .ui-menubar-item .ui-button:first" );
+
+	/* Make the qunit fixture visible if we're debugging this test*/
+  if ( debugDelay ) {
+		$('<link rel="stylesheet" href="../../../themes/base/jquery.ui.all.css" />').appendTo("head");
+		$( "#qunit-fixture" ).css({ right: "300px", top: "300px", left:0 });
+	}
+
+	setTimeout(function(){
+		firstMenuItem[ 0 ].focus();
+
+		function postFocus(){
+			ok( !firstMenuItem.hasClass( "ui-state-focus" ), "The manually focused item should no longer have focus after TAB" );
+			setTimeout( start );
+		};
+
+		setTimeout(function() {
+			ok( firstMenuItem.hasClass( "ui-state-focus" ), "Should have focus class" );
+			equal( document.activeElement, firstMenuItem, "Focus set on first menuItem" );
+			$( document.activeElement ).simulate( "keydown", { keyCode: $.ui.keyCode.TAB } );
+			setTimeout( postFocus );
+		});
+
+	}, debugDelay );
+
+} );
+
+
 })( jQuery );

ui/jquery.ui.menubar.js

@@ -43,6 +43,9 @@ $.widget( "ui.menubar", {
 
 		// Keep track of open submenus
 		this.openSubmenus = 0;
+
+		// Scorched earth: NOTHING can be tabbed to
+		this.menuItems.find("*").slice(1).attr("tabindex", -1);
 	},
 
 	_initializeMenubarsBoundElement: function() {
@@ -65,6 +68,7 @@ $.widget( "ui.menubar", {
 				}
 			},
 			focusin: function() {
+        this._disableTabIndexOnFirstMenuItem();
 				clearTimeout( menubar.closeTimer );
 			},
 			focusout: function( event ) {
@@ -105,7 +109,7 @@ $.widget( "ui.menubar", {
 			isLastElement = ( index === ( collectionLength - 1 ) );
 
 		if ( isFirstElement ) {
-			$menuItem.data( "prevMenuItem", $( this.menuItems[collectionLength - 1])  );
+			$menuItem.data( "prevMenuItem", $( this.menuItems[collectionLength - 1]) );
 			$menuItem.data( "nextMenuItem", $( this.menuItems[index+1])  );
 		} else if ( isLastElement ) {
 			$menuItem.data( "nextMenuItem", $( this.menuItems[0])  );
@@ -171,6 +175,7 @@ $.widget( "ui.menubar", {
 
 		this._on( subMenus, {
 			keydown: function( event ) {
+        $(event.target).attr("tabIndex", 1);
 				var parentButton,
 					menu = $( this );
 				if ( menu.is(":hidden") ) {
@@ -192,6 +197,7 @@ $.widget( "ui.menubar", {
 					}
 
 					event.preventDefault();
+        $(event.target).attr("tabIndex", -1);
 					break;
 				case $.ui.keyCode.RIGHT:
 					this.next( event );
@@ -206,15 +212,8 @@ $.widget( "ui.menubar", {
 	},
 
 	_initializeItem: function( $anItem, menubar ) {
-		//only the first item is eligible to receive the focus
 		var menuItemHasSubMenu = $anItem.data("parentMenuItem").data("hasSubMenu");
 
-		// Only the first item is tab-able
-		if ( menubar.items.length === 1 ) {
-			$anItem.attr( "tabindex", 1 );
-		} else {
-			$anItem.attr( "tabIndex", -1 );
-		}
 
 		this._focusable( this.items );
 		this._hoverable( this.items );
@@ -239,10 +238,12 @@ $.widget( "ui.menubar", {
 
 	__applyMouseAndKeyboardBehaviorForMenuItem: function( $anItem, menubar ) {
 		menubar._on( $anItem, {
-			focus:  function(){
+			focus:	function(){
+        $anItem.attr("tabIndex", 1);
 				$anItem.addClass("ui-state-focus");
 			},
 			focusout:  function(){
+        $anItem.attr("tabIndex", -1);
 				$anItem.removeClass("ui-state-focus");
 			}
 		} );
@@ -287,7 +288,6 @@ $.widget( "ui.menubar", {
 					this._open( event, menu );
 				}
 			};
-
 		menubar._on( input, {
 			click: mouseBehaviorCallback,
 			focus: mouseBehaviorCallback,
@@ -312,6 +312,9 @@ $.widget( "ui.menubar", {
 				this.next( event );
 				event.preventDefault();
 				break;
+			case $.ui.keyCode.TAB:
+        event.stopPropagation();
+				break;
 			}
 		};
 
@@ -439,7 +442,7 @@ $.widget( "ui.menubar", {
 		}
 
 		// set tabIndex -1 to have the button skipped on shift-tab when menu is open (it gets focus)
-		button = menuItem.addClass("ui-state-active").attr( "tabIndex", -1 );
+		button = menuItem.addClass("ui-state-active");
 
 		this.active = menu
 			.show()
@@ -501,7 +504,7 @@ $.widget( "ui.menubar", {
 				this._submenuless_open( event, nextMenuItem );
 			}
 		} else {
-			closestMenuItem.find(".ui-button").attr( "tabindex", -1 );
+			closestMenuItem.find(".ui-button");
 			focusableTarget.focus();
 		}
 	},
@@ -539,8 +542,12 @@ $.widget( "ui.menubar", {
 		menu.parent(".ui-menubar-item").removeClass("ui-state-active");
 	},
 
+  _disableTabIndexOnFirstMenuItem: function() {
+    this.items[0].attr( "tabIndex", -1 );
+  },
+
 	_reenableTabIndexOnFirstMenuItem: function() {
-		$(this.menuItems[0]).find(".ui-widget").attr( "tabindex", 1 );
+    this.items[0].attr( "tabIndex", 1 );
 	}
 
 });