From f44c2d3efdaf314d505a4d608afd94ed61d6a3d6 Mon Sep 17 00:00:00 2001
From: "aaronleventhal@moonset.net" <aaronleventhal@moonset.net>
Date: Tue, 22 Jan 2008 07:32:20 -0800
Subject: [PATCH] Bug 407359. Fix accessibility issues with popups in main
 screen UI. r=marcoz, enn, surkov, mano, ui=mconnor, a=blocking1.9

---
 accessible/public/nsIAccessibleProvider.idl   | 43 ++++++----
 accessible/public/nsIAccessibleRole.idl       |  4 +-
 accessible/src/atk/nsRoleMap.h                |  2 +-
 .../src/base/nsAccessibilityService.cpp       |  5 ++
 accessible/src/base/nsAccessibilityService.h  |  2 +-
 accessible/src/base/nsCaretAccessible.cpp     |  2 +-
 accessible/src/base/nsOuterDocAccessible.cpp  |  2 +-
 accessible/src/base/nsRootAccessible.cpp      | 62 ++++++++------
 accessible/src/mac/nsRoleMap.h                |  2 +-
 accessible/src/msaa/nsRoleMap.h               |  6 +-
 accessible/src/xul/nsXULAlertAccessible.cpp   |  8 ++
 accessible/src/xul/nsXULAlertAccessible.h     |  1 +
 accessible/src/xul/nsXULMenuAccessible.cpp    | 22 +++--
 accessible/src/xul/nsXULSelectAccessible.cpp  | 18 +++-
 accessible/src/xul/nsXULTextAccessible.cpp    | 20 ++---
 browser/base/content/browser-places.js        |  7 +-
 browser/base/content/browser.js               | 28 ++++--
 browser/base/content/browser.xul              | 15 ++--
 .../themes/gnomestripe/browser/browser.css    |  5 ++
 browser/themes/pinstripe/browser/browser.css  |  5 ++
 browser/themes/winstripe/browser/browser.css  |  5 ++
 toolkit/content/widgets/autocomplete.xml      | 16 +++-
 toolkit/content/widgets/popup.xml             | 85 ++++++++++++++++---
 toolkit/content/widgets/textbox.xml           |  6 +-
 24 files changed, 260 insertions(+), 111 deletions(-)

diff --git a/accessible/public/nsIAccessibleProvider.idl b/accessible/public/nsIAccessibleProvider.idl
index 9e6db44fc81d..3f7b22774b1d 100644
--- a/accessible/public/nsIAccessibleProvider.idl
+++ b/accessible/public/nsIAccessibleProvider.idl
@@ -52,6 +52,12 @@ interface nsIAccessibleProvider : nsISupports
    * Constants set of common use.
    */
 
+  /** Do not create an accessible for this object
+   * This is useful if an ancestor binding already implements nsIAccessibleProvider,
+   * but no accessible is desired for the inheriting binding
+   */
+  const long NoAccessible = 0;
+
   /** For elements that spawn a new document. For example now it is used by
     <xul:iframe>, <xul:browser> and <xul:editor>. */
   const long OuterDoc = 0x00000001;
@@ -78,32 +84,33 @@ interface nsIAccessibleProvider : nsISupports
   const long XULMenuitem = 0x0000100E;
   const long XULMenupopup = 0x0000100F;
   const long XULMenuSeparator = 0x00001010;
-  const long XULProgressMeter = 0x00001011;
-  const long XULScale = 0x00001012;
-  const long XULStatusBar = 0x00001013;
-  const long XULRadioButton = 0x00001014;
-  const long XULRadioGroup = 0x00001015;
+  const long XULPane    = 0x00001011;
+  const long XULProgressMeter = 0x00001012;
+  const long XULScale = 0x00001013;
+  const long XULStatusBar = 0x00001014;
+  const long XULRadioButton = 0x00001015;
+  const long XULRadioGroup = 0x00001016;
 
   /** The single tab in a dialog or tabbrowser/editor interface */
-  const long XULTab = 0x00001016;
+  const long XULTab = 0x00001017;
 
   /** A combination of a tabs object and a tabpanels object */
-  const long XULTabBox = 0x00001017;
+  const long XULTabBox = 0x00001018;
 
   /** The collection of tab objects, useable in the TabBox and independant of
    as well */
-  const long XULTabs = 0x00001018;
+  const long XULTabs = 0x00001019;
 
-  const long XULText             = 0x00001019;
-  const long XULTextBox          = 0x0000101A;
-  const long XULThumb            = 0x0000101B;
-  const long XULTree             = 0x0000101C;
-  const long XULTreeColumns      = 0x0000101D;
-  const long XULTreeColumnItem   = 0x0000101E;
-  const long XULToolbar          = 0x0000101F;
-  const long XULToolbarSeparator = 0x00001020;
-  const long XULTooltip          = 0x00001021;
-  const long XULToolbarButton    = 0x00001022;
+  const long XULText             = 0x0000101A;
+  const long XULTextBox          = 0x0000101B;
+  const long XULThumb            = 0x0000101C;
+  const long XULTree             = 0x0000101D;
+  const long XULTreeColumns      = 0x0000101E;
+  const long XULTreeColumnItem   = 0x0000101F;
+  const long XULToolbar          = 0x00001020;
+  const long XULToolbarSeparator = 0x00001021;
+  const long XULTooltip          = 0x00001022;
+  const long XULToolbarButton    = 0x00001023;
 
 
   /**
diff --git a/accessible/public/nsIAccessibleRole.idl b/accessible/public/nsIAccessibleRole.idl
index 8ccbdf191b69..dc43574512c3 100755
--- a/accessible/public/nsIAccessibleRole.idl
+++ b/accessible/public/nsIAccessibleRole.idl
@@ -110,9 +110,9 @@ interface nsIAccessibleRole : nsISupports
   const unsigned long ROLE_WINDOW = 9;
 
   /**
-   * XXX: document this.
+   * A sub-document (<frame> or <iframe>)
    */
-  const unsigned long ROLE_CLIENT = 10;
+  const unsigned long ROLE_INTERNAL_FRAME = 10;
 
   /**
    * Represents a menu, which presents a list of options from which the user can
diff --git a/accessible/src/atk/nsRoleMap.h b/accessible/src/atk/nsRoleMap.h
index 98da57051dc8..bdcf2138605d 100644
--- a/accessible/src/atk/nsRoleMap.h
+++ b/accessible/src/atk/nsRoleMap.h
@@ -56,7 +56,7 @@ static const PRUint32 atkRoleMap[] = {
     ATK_ROLE_UNKNOWN,             // nsIAccessibleRole::ROLE_CARET                7
     ATK_ROLE_ALERT,               // nsIAccessibleRole::ROLE_ALERT                8
     ATK_ROLE_WINDOW,              // nsIAccessibleRole::ROLE_WINDOW               9
-    ATK_ROLE_PANEL,               // nsIAccessibleRole::ROLE_CLIENT               10
+    ATK_ROLE_INTERNAL_FRAME,      // nsIAccessibleRole::ROLE_INTERNAL_FRAME       10
     ATK_ROLE_MENU,                // nsIAccessibleRole::ROLE_MENUPOPUP            11
     ATK_ROLE_MENU_ITEM,           // nsIAccessibleRole::ROLE_MENUITEM             12
     ATK_ROLE_TOOL_TIP,            // nsIAccessibleRole::ROLE_TOOLTIP              13
diff --git a/accessible/src/base/nsAccessibilityService.cpp b/accessible/src/base/nsAccessibilityService.cpp
index 9f77c6fd2ebb..db3684536049 100644
--- a/accessible/src/base/nsAccessibilityService.cpp
+++ b/accessible/src/base/nsAccessibilityService.cpp
@@ -1630,6 +1630,8 @@ nsresult nsAccessibilityService::GetAccessibleByType(nsIDOMNode *aNode,
   switch (type)
   {
 #ifdef MOZ_XUL
+    case nsIAccessibleProvider::NoAccessible:
+      return NS_OK;
     // XUL controls
     case nsIAccessibleProvider::XULAlert:
       *aAccessible = new nsXULAlertAccessible(aNode, weakShell);
@@ -1714,6 +1716,9 @@ nsresult nsAccessibilityService::GetAccessibleByType(nsIDOMNode *aNode,
     case nsIAccessibleProvider::XULMenuSeparator:
       *aAccessible = new nsXULMenuSeparatorAccessible(aNode, weakShell);
       break;
+    case nsIAccessibleProvider::XULPane:
+      *aAccessible = new nsEnumRoleAccessible(aNode, weakShell, nsIAccessibleRole::ROLE_PANE);
+      break;
     case nsIAccessibleProvider::XULProgressMeter:
       *aAccessible = new nsXULProgressMeterAccessible(aNode, weakShell);
       break;
diff --git a/accessible/src/base/nsAccessibilityService.h b/accessible/src/base/nsAccessibilityService.h
index 62af880b804b..1de8f0dfe705 100644
--- a/accessible/src/base/nsAccessibilityService.h
+++ b/accessible/src/base/nsAccessibilityService.h
@@ -152,7 +152,7 @@ static const char kRoleNames[][20] = {
   "caret",               //ROLE_CARET
   "alert",               //ROLE_ALERT
   "window",              //ROLE_WINDOW
-  "client",              //ROLE_CLIENT
+  "internal frame",      //ROLE_INTERNAL_FRAME
   "menupopup",           //ROLE_MENUPOPUP
   "menuitem",            //ROLE_MENUITEM
   "tooltip",             //ROLE_TOOLTIP
diff --git a/accessible/src/base/nsCaretAccessible.cpp b/accessible/src/base/nsCaretAccessible.cpp
index b8bfda117e72..62cb28857735 100644
--- a/accessible/src/base/nsCaretAccessible.cpp
+++ b/accessible/src/base/nsCaretAccessible.cpp
@@ -171,7 +171,7 @@ NS_IMETHODIMP nsCaretAccessible::NotifySelectionChanged(nsIDOMDocument *aDoc, ns
   nsIPresShell *presShell = doc->GetPrimaryShell();
   NS_ENSURE_TRUE(presShell, NS_OK);
 
-  // Get first nnsIAccessibleText in parent chain and fire caret-move, selection-change event for it
+  // Get first nsIAccessibleText in parent chain and fire caret-move, selection-change event for it
   nsCOMPtr<nsIAccessible> accessible;
   nsIAccessibilityService *accService = mRootAccessible->GetAccService();
   NS_ENSURE_TRUE(accService, NS_ERROR_FAILURE);
diff --git a/accessible/src/base/nsOuterDocAccessible.cpp b/accessible/src/base/nsOuterDocAccessible.cpp
index 9a5cb0663254..f703cee63011 100644
--- a/accessible/src/base/nsOuterDocAccessible.cpp
+++ b/accessible/src/base/nsOuterDocAccessible.cpp
@@ -74,7 +74,7 @@ NS_IMETHODIMP nsOuterDocAccessible::GetName(nsAString& aName)
 /* unsigned long getRole (); */
 NS_IMETHODIMP nsOuterDocAccessible::GetRole(PRUint32 *aRole)
 {
-  *aRole = nsIAccessibleRole::ROLE_CLIENT;
+  *aRole = nsIAccessibleRole::ROLE_INTERNAL_FRAME;
   return NS_OK;
 }
 
diff --git a/accessible/src/base/nsRootAccessible.cpp b/accessible/src/base/nsRootAccessible.cpp
index 7614afdf17a9..236df089187e 100644
--- a/accessible/src/base/nsRootAccessible.cpp
+++ b/accessible/src/base/nsRootAccessible.cpp
@@ -597,6 +597,9 @@ nsresult nsRootAccessible::HandleEventWithTarget(nsIDOMEvent* aEvent,
   aEvent->GetType(eventType);
   nsAutoString localName;
   aTargetNode->GetLocalName(localName);
+#ifdef MOZ_XUL
+  PRBool isTree = localName.EqualsLiteral("tree");
+#endif
 #ifdef DEBUG_A11Y
   // Very useful for debugging, please leave this here.
   if (eventType.EqualsLiteral("AlertActive")) {
@@ -642,8 +645,9 @@ nsresult nsRootAccessible::HandleEventWithTarget(nsIDOMEvent* aEvent,
     return NS_OK;
   }
 
+#ifdef MOZ_XUL
   if (eventType.EqualsLiteral("TreeViewChanged")) { // Always asynch, always from user input
-    if (!localName.EqualsLiteral("tree"))
+    if (!isTree)
       return NS_OK;
 
     nsCOMPtr<nsIContent> treeContent = do_QueryInterface(aTargetNode);
@@ -651,6 +655,29 @@ nsresult nsRootAccessible::HandleEventWithTarget(nsIDOMEvent* aEvent,
     return accService->InvalidateSubtreeFor(eventShell, treeContent,
                                             nsIAccessibleEvent::EVENT_ASYNCH_SIGNIFICANT_CHANGE);
   }
+#endif
+
+  if (eventType.EqualsLiteral("popuphiding")) {
+    // If accessible focus was on or inside popup that closes,
+    // then restore it to true current focus.
+    // This is the case when we've been getting DOMMenuItemActive events
+    // inside of a combo box that closes. The real focus is on the combo box.
+    // It's also the case when a popup gets focus in ATK -- when it closes
+    // we need to fire an event to restore focus to where it was
+    if (!gLastFocusedNode) {
+      return NS_OK;
+    }
+    if (gLastFocusedNode != aTargetNode) {
+      // Was not focused on popup
+      nsCOMPtr<nsIDOMNode> parentOfFocus;
+      gLastFocusedNode->GetParentNode(getter_AddRefs(parentOfFocus));
+      if (parentOfFocus != aTargetNode) {
+        return NS_OK;  // And was not focused on an item inside the popup
+      }
+    }
+    // Focus was on or inside of a popup that's being hidden
+    FireCurrentFocusEvent();
+  }
 
   nsCOMPtr<nsIAccessible> accessible;
   accService->GetAccessibleInShell(aTargetNode, eventShell,
@@ -659,8 +686,9 @@ nsresult nsRootAccessible::HandleEventWithTarget(nsIDOMEvent* aEvent,
   if (!privAcc)
     return NS_OK;
 
+#ifdef MOZ_XUL
   if (eventType.EqualsLiteral("TreeRowCountChanged")) {
-    if (!localName.EqualsLiteral("tree"))
+    if (!isTree)
       return NS_OK;
 
     nsCOMPtr<nsIDOMDataContainerEvent> dataEvent(do_QueryInterface(aEvent));
@@ -685,6 +713,7 @@ nsresult nsRootAccessible::HandleEventWithTarget(nsIDOMEvent* aEvent,
 
     return treeAccCache->InvalidateCache(index, count);
   }
+#endif
 
   if (eventType.EqualsLiteral("RadioStateChange")) {
     PRUint32 state = State(accessible);
@@ -723,7 +752,7 @@ nsresult nsRootAccessible::HandleEventWithTarget(nsIDOMEvent* aEvent,
   nsCOMPtr<nsIAccessible> treeItemAccessible;
 #ifdef MOZ_XUL
   // If it's a tree element, need the currently selected item
-  if (localName.EqualsLiteral("tree")) {
+  if (isTree) {
     nsCOMPtr<nsIDOMXULMultiSelectControlElement> multiSelect =
       do_QueryInterface(aTargetNode);
     if (multiSelect) {
@@ -847,28 +876,6 @@ nsresult nsRootAccessible::HandleEventWithTarget(nsIDOMEvent* aEvent,
       nsAccUtils::FireAccEvent(event, accessible);
     }
   }
-
-  else if (eventType.EqualsLiteral("popuphiding")) {
-    // If accessible focus was on or inside popup that closes,
-    // then restore it to true current focus.
-    // This is the case when we've been getting DOMMenuItemActive events
-    // inside of a combo box that closes. The real focus is on the combo box.
-    // It's also the case when a popup gets focus in ATK -- when it closes
-    // we need to fire an event to restore focus to where it was
-    if (!gLastFocusedNode) {
-      return NS_OK;
-    }
-    if (gLastFocusedNode != aTargetNode) {
-      // Was not focused on popup
-      nsCOMPtr<nsIDOMNode> parentOfFocus;
-      gLastFocusedNode->GetParentNode(getter_AddRefs(parentOfFocus));
-      if (parentOfFocus != aTargetNode) {
-        return NS_OK;  // And was not focused on an item inside the popup
-      }
-    }
-    // Focus was on or inside of a popup that's being hidden
-    FireCurrentFocusEvent();
-  }
   else if (eventType.EqualsLiteral("DOMMenuInactive")) {
     if (Role(accessible) == nsIAccessibleRole::ROLE_MENUPOPUP) {
       nsAccUtils::FireAccEvent(nsIAccessibleEvent::EVENT_MENUPOPUP_END,
@@ -877,6 +884,11 @@ nsresult nsRootAccessible::HandleEventWithTarget(nsIDOMEvent* aEvent,
   }
   else if (eventType.EqualsLiteral("DOMMenuItemActive")) {
     if (!treeItemAccessible) {
+#ifdef MOZ_XUL
+      if (isTree) {
+        return NS_OK; // Tree with nothing selected
+      }
+#endif
       nsCOMPtr<nsPIAccessNode> menuAccessNode = do_QueryInterface(accessible);
       NS_ENSURE_TRUE(menuAccessNode, NS_ERROR_FAILURE);
       nsIFrame* menuFrame = menuAccessNode->GetFrame();
diff --git a/accessible/src/mac/nsRoleMap.h b/accessible/src/mac/nsRoleMap.h
index ba406ab21a90..2802537c9aed 100644
--- a/accessible/src/mac/nsRoleMap.h
+++ b/accessible/src/mac/nsRoleMap.h
@@ -52,7 +52,7 @@ static const NSString* AXRoles [] = {
   NSAccessibilityUnknownRole,                   // ROLE_CARET. unused on OS X
   NSAccessibilityWindowRole,                    // ROLE_ALERT
   NSAccessibilityWindowRole,                    // ROLE_WINDOW. irrelevant on OS X; all window a11y is handled by the system.
-  @"AXWebArea",                                 // ROLE_CLIENT
+  @"AXWebArea",                                 // ROLE_INTERNAL_FRAME
   NSAccessibilityMenuRole,                      // ROLE_MENUPOPUP. the parent of menuitems
   NSAccessibilityMenuItemRole,                  // ROLE_MENUITEM.
   @"AXHelpTag",                                 // ROLE_TOOLTIP. 10.4+ only, so we re-define the constant.
diff --git a/accessible/src/msaa/nsRoleMap.h b/accessible/src/msaa/nsRoleMap.h
index edaab6fd906c..94c45b40d6c7 100644
--- a/accessible/src/msaa/nsRoleMap.h
+++ b/accessible/src/msaa/nsRoleMap.h
@@ -95,8 +95,8 @@ static const WindowsRoleMapItem gWindowsRoleMap[] = {
   // nsIAccessibleRole::ROLE_WINDOW
   { ROLE_SYSTEM_WINDOW, ROLE_SYSTEM_WINDOW },
 
-  // nsIAccessibleRole::ROLE_CLIENT
-  { USE_ROLE_STRING, IA2_ROLE_UNKNOWN},
+  // nsIAccessibleRole::ROLE_INTERNAL_FRAME
+  { USE_ROLE_STRING, IA2_ROLE_INTERNAL_FRAME},
 
   // nsIAccessibleRole::ROLE_MENUPOPUP
   { ROLE_SYSTEM_MENUPOPUP, ROLE_SYSTEM_MENUPOPUP },
@@ -429,7 +429,7 @@ static const WindowsRoleMapItem gWindowsRoleMap[] = {
   { ROLE_SYSTEM_LISTITEM, ROLE_SYSTEM_LISTITEM },
   
   // nsIAccessibleRole::ROLE_RICH_OPTION
-  { ROLE_SYSTEM_LIST, ROLE_SYSTEM_LIST },
+  { ROLE_SYSTEM_LISTITEM, ROLE_SYSTEM_LISTITEM },
   
   // nsIAccessibleRole::ROLE_LISTBOX
   { ROLE_SYSTEM_LIST, ROLE_SYSTEM_LIST },
diff --git a/accessible/src/xul/nsXULAlertAccessible.cpp b/accessible/src/xul/nsXULAlertAccessible.cpp
index f757e8e694be..83bda8458827 100644
--- a/accessible/src/xul/nsXULAlertAccessible.cpp
+++ b/accessible/src/xul/nsXULAlertAccessible.cpp
@@ -64,3 +64,11 @@ nsXULAlertAccessible::GetState(PRUint32 *aState, PRUint32 *aExtraState)
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsXULAlertAccessible::GetName(nsAString& aName)
+{
+  // Screen readers need to read contents of alert, not the accessible name.
+  // If we have both some screen readers will read the alert twice.
+  aName.Truncate();
+  return NS_OK;
+}
diff --git a/accessible/src/xul/nsXULAlertAccessible.h b/accessible/src/xul/nsXULAlertAccessible.h
index b1b78acd45fe..9081f26f446a 100644
--- a/accessible/src/xul/nsXULAlertAccessible.h
+++ b/accessible/src/xul/nsXULAlertAccessible.h
@@ -50,6 +50,7 @@ public:
   NS_DECL_ISUPPORTS_INHERITED
   NS_IMETHOD GetRole(PRUint32 *aRole);
   NS_IMETHOD GetState(PRUint32 *aState, PRUint32 *aExtraState);
+  NS_IMETHOD GetName(nsAString& aName);
 };
 
 #endif  
diff --git a/accessible/src/xul/nsXULMenuAccessible.cpp b/accessible/src/xul/nsXULMenuAccessible.cpp
index e6015e821c24..4043eca49295 100644
--- a/accessible/src/xul/nsXULMenuAccessible.cpp
+++ b/accessible/src/xul/nsXULMenuAccessible.cpp
@@ -486,7 +486,9 @@ NS_IMETHODIMP nsXULMenuitemAccessible::GetRole(PRUint32 *aRole)
     return NS_OK;
   }
 
-  if (mParent && Role(mParent) == nsIAccessibleRole::ROLE_COMBOBOX_LIST) {
+  nsCOMPtr<nsIAccessible> parent;
+  GetParent(getter_AddRefs(parent));
+  if (parent && Role(parent) == nsIAccessibleRole::ROLE_COMBOBOX_LIST) {
     *aRole = nsIAccessibleRole::ROLE_COMBOBOX_OPTION;
     return NS_OK;
   }
@@ -716,14 +718,18 @@ NS_IMETHODIMP nsXULMenupopupAccessible::GetRole(PRUint32 *aRole)
   if (!content) {
     return NS_ERROR_FAILURE;
   }
-  if ((mParent && Role(mParent) == nsIAccessibleRole::ROLE_COMBOBOX) ||
-      content->AttrValueIs(kNameSpaceID_None, nsAccessibilityAtoms::type,
-                           nsAccessibilityAtoms::autocomplete, eIgnoreCase)) {
-    *aRole = nsIAccessibleRole::ROLE_COMBOBOX_LIST;
-  }
-  else {
-    *aRole = nsIAccessibleRole::ROLE_MENUPOPUP;
+  nsCOMPtr<nsIAccessible> parent;
+  GetParent(getter_AddRefs(parent));
+  if (parent) {
+    // Some widgets like the search bar have several popups, owned by buttons
+    PRUint32 role = Role(parent);
+    if (role == nsIAccessibleRole::ROLE_COMBOBOX ||
+        role == nsIAccessibleRole::ROLE_PUSHBUTTON) {
+      *aRole = nsIAccessibleRole::ROLE_COMBOBOX_LIST;
+      return NS_OK;
+    }
   }
+  *aRole = nsIAccessibleRole::ROLE_MENUPOPUP;
   return NS_OK;
 }
 
diff --git a/accessible/src/xul/nsXULSelectAccessible.cpp b/accessible/src/xul/nsXULSelectAccessible.cpp
index d8622b223205..76ea5b58a24d 100644
--- a/accessible/src/xul/nsXULSelectAccessible.cpp
+++ b/accessible/src/xul/nsXULSelectAccessible.cpp
@@ -41,6 +41,7 @@
 #include "nsAccessibilityService.h"
 #include "nsIContent.h"
 #include "nsIDOMXULMenuListElement.h"
+#include "nsIDOMXULPopupElement.h"
 #include "nsIDOMXULSelectCntrlItemEl.h"
 #include "nsIDOMXULSelectCntrlEl.h"
 #include "nsIDOMXULTextboxElement.h"
@@ -194,9 +195,20 @@ NS_IMETHODIMP nsXULListboxAccessible::GetValue(nsAString& _retval)
   return NS_ERROR_FAILURE;
 }
 
-NS_IMETHODIMP nsXULListboxAccessible::GetRole(PRUint32 *_retval)
+NS_IMETHODIMP nsXULListboxAccessible::GetRole(PRUint32 *aRole)
 {
-  *_retval = nsIAccessibleRole::ROLE_LIST;
+  nsCOMPtr<nsIContent> content = do_QueryInterface(mDOMNode);
+  if (content) {
+    // A richlistbox is used with the new autocomplete URL bar,
+    // and has a parent popup <panel>
+    nsCOMPtr<nsIDOMXULPopupElement> xulPopup =
+      do_QueryInterface(content->GetParent());
+    if (xulPopup) {
+      *aRole = nsIAccessibleRole::ROLE_COMBOBOX_LIST;
+      return NS_OK;
+    }
+  }
+  *aRole = nsIAccessibleRole::ROLE_LIST;
   return NS_OK;
 }
 
@@ -250,6 +262,8 @@ NS_IMETHODIMP nsXULListitemAccessible::GetRole(PRUint32 *aRole)
 {
   if (mIsCheckbox)
     *aRole = nsIAccessibleRole::ROLE_CHECKBUTTON;
+  else if (mParent && Role(mParent) == nsIAccessibleRole::ROLE_COMBOBOX_LIST)
+    *aRole = nsIAccessibleRole::ROLE_COMBOBOX_OPTION;
   else
     *aRole = nsIAccessibleRole::ROLE_RICH_OPTION;
   return NS_OK;
diff --git a/accessible/src/xul/nsXULTextAccessible.cpp b/accessible/src/xul/nsXULTextAccessible.cpp
index bf413d8df6b4..21c2a64794f7 100644
--- a/accessible/src/xul/nsXULTextAccessible.cpp
+++ b/accessible/src/xul/nsXULTextAccessible.cpp
@@ -61,13 +61,9 @@ NS_IMETHODIMP nsXULTextAccessible::GetName(nsAString& aName)
   if (!content) {
     return NS_ERROR_FAILURE;  // Node shut down
   }
-  if (!content->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::value,
-                        aName)) {
-    // if the value doesn't exist, flatten the inner content as the name (for descriptions)
-    return AppendFlatStringFromSubtree(content, &aName);
-  }
-  // otherwise, use the value attribute as the name (for labels)
-  return NS_OK;
+  // if the value attr doesn't exist, the screen reader must get the accessible text
+  // from the accessible text interface or from the children
+  return content->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::value, aName);
 }
 
 NS_IMETHODIMP
@@ -176,13 +172,9 @@ NS_IMETHODIMP nsXULLinkAccessible::GetName(nsAString& aName)
 
 NS_IMETHODIMP nsXULLinkAccessible::GetRole(PRUint32 *aRole)
 {
-  if (mIsLink) {
-    *aRole = nsIAccessibleRole::ROLE_LINK;
-  } else {
-    // default to calling the link a button; might have javascript
-    *aRole = nsIAccessibleRole::ROLE_PUSHBUTTON;
-  }
-  // should there be a third case where it becomes just text?
+  // We used to say ROLE_BUTTON if there was no href, but then screen readers
+  // would tell users to hit the space bar for activation, which is wrong for a link
+  *aRole = nsIAccessibleRole::ROLE_LINK;
   return NS_OK;
 }
 
diff --git a/browser/base/content/browser-places.js b/browser/base/content/browser-places.js
index 80350f165e1f..d4241e752522 100644
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -97,11 +97,8 @@ var PlacesCommandHook = {
         break;
       case "keypress":
         if (aEvent.keyCode == KeyEvent.DOM_VK_ESCAPE ||
-            aEvent.keyCode == KeyEvent.DOM_VK_RETURN) {
-          // focus the content area and hide the panel
-          window.content.focus();
-          this.panel.hidePopup();
-        }
+            aEvent.keyCode == KeyEvent.DOM_VK_RETURN)
+          this.panel.hidePopup(); // hide the panel
         break;
     }
   },
diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js
index 252f8acce677..963e66676af5 100644
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -1503,13 +1503,20 @@ function loadOneOrMoreURIs(aURIString)
   }
 }
 
-function openLocation()
+function focusAndSelectUrlBar()
 {
   if (gURLBar && isElementVisible(gURLBar) && !gURLBar.readOnly) {
     gURLBar.focus();
     gURLBar.select();
-    return;
+    return true;
   }
+  return false;
+}
+
+function openLocation()
+{
+  if (focusAndSelectUrlBar())
+    return;
 #ifdef XP_MACOSX
   if (window.location.href != getBrowserURL()) {
     var win = getTopWin();
@@ -5751,16 +5758,23 @@ IdentityHandler.prototype = {
     this._identityPopupContentSupp.textContent = supplemental;
     this._identityPopupContentVerif.textContent = verifier;
   },
-  
+
+  hideIdentityPopup : function() {
+    this._identityPopup.hidePopup();
+  },
+
   /**
    * Click handler for the identity-box element in primary chrome.  
    */
-  handleIdentityClick : function(event) {
+  handleIdentityButtonEvent : function(event) {
+  
     event.stopPropagation();
+ 
+    if ((event.type == "click" && event.button != 0) ||
+        (event.type == "keypress" && event.charCode != KeyEvent.DOM_VK_SPACE &&
+         event.keyCode != KeyEvent.DOM_VK_RETURN))
+      return; // Left click, space or enter only
 
-    if (event.button != 0)
-      return; // We only want left-clicks
-        
     // Make sure that the display:none style we set in xul is removed now that
     // the popup is actually needed
     this._identityPopup.hidden = false;
diff --git a/browser/base/content/browser.xul b/browser/base/content/browser.xul
index f50a32aacf4a..1ec428ad596a 100644
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -106,7 +106,8 @@
     <panel type="autocomplete-richlistbox" chromedir="&locale.dir;" id="PopupAutoCompleteRichResult" noautofocus="true" hidden="true"/>
 
     <panel id="editBookmarkPanel" orient="vertical" hidden="true"
-           onpopupshown="PlacesCommandHook.editBookmarkPanelShown();">
+           onpopupshown="PlacesCommandHook.editBookmarkPanelShown();"
+           label="&bookmarkPageCmd.label;">
       <vbox id="editBookmarkPanelContent" flex="1"/>
       <hbox flex="1">
         <spacer flex="1"/>
@@ -145,12 +146,14 @@
     <popup id="placesContext"/>
 
     <!-- Popup for site identity information -->
-    <panel id="identity-popup" position="after_start" hidden="true" noautofocus="true">
+    <panel id="identity-popup" position="after_start" hidden="true" noautofocus="true"
+           onpopupshown="document.getElementById('identity-popup-more-info-link').focus();"
+           onpopuphidden="focusAndSelectUrlBar();" norestorefocus="true">
       <hbox id="identity-popup-container" align="top">
         <image id="identity-popup-icon"/>
         <vbox id="identity-popup-content-box">
           <!-- Title Bar -->
-          <label id="identity-popup-title"/>
+          <label id="identity-popup-title" control="identity-popup"/>
           <!-- Content area -->
           <description id="identity-popup-content"/>
           <description id="identity-popup-content-supplemental"/>
@@ -167,6 +170,7 @@
           <label id="identity-popup-more-info-link"
                  class="text-link plain"
                  value="&identity.moreInfoLinkText;"
+                 onblur="getIdentityHandler().hideIdentityPopup();"
                  onclick="getIdentityHandler().handleMoreInfoClick(event);"/>
         </vbox>
       </hbox>
@@ -269,8 +273,9 @@
                  pageproxystate="invalid">
           <!-- Use onclick instead of normal popup= syntax since the popup
                code fires onmousedown, and hence eats our favicon drag events -->
-          <box id="identity-box"
-               onclick="getIdentityHandler().handleIdentityClick(event);">
+          <box id="identity-box" role="button"
+               onclick="getIdentityHandler().handleIdentityButtonEvent(event);"
+               onkeypress="getIdentityHandler().handleIdentityButtonEvent(event);">
             <hbox align="center">
               <deck id="page-proxy-deck" onclick="PageProxyClickHandler(event);">
                 <image id="page-proxy-button"
diff --git a/browser/themes/gnomestripe/browser/browser.css b/browser/themes/gnomestripe/browser/browser.css
index e09d9337ca49..08f7dc3c425f 100644
--- a/browser/themes/gnomestripe/browser/browser.css
+++ b/browser/themes/gnomestripe/browser/browser.css
@@ -846,6 +846,11 @@ toolbar[iconsize="small"] #paste-button[disabled="true"] {
 #identity-box {
   background-color: -moz-dialog;
   -moz-border-end: 1px solid ThreeDShadow;
+  -moz-user-focus: normal;
+}
+
+#identity-box:focus {
+  outline: 1px dotted -moz-DialogText;
 }
 
 #identity-icon-label {
diff --git a/browser/themes/pinstripe/browser/browser.css b/browser/themes/pinstripe/browser/browser.css
index 6ff3abd1dbd2..0ca68618b2b0 100755
--- a/browser/themes/pinstripe/browser/browser.css
+++ b/browser/themes/pinstripe/browser/browser.css
@@ -1635,6 +1635,11 @@ toolbarbutton.bookmark-item[dragover="true"][open="true"] {
   border-right: 1px solid #888;
   background-color: white;
   opacity: 0.9;
+  -moz-user-focus: normal;
+}
+
+#identity-box:focus {
+  outline: 1.4pt solid -moz-mac-focusring;
 }
 
 #identity-box:hover {
diff --git a/browser/themes/winstripe/browser/browser.css b/browser/themes/winstripe/browser/browser.css
index 482bed05fd36..f041a53f5599 100644
--- a/browser/themes/winstripe/browser/browser.css
+++ b/browser/themes/winstripe/browser/browser.css
@@ -1592,6 +1592,11 @@ toolbarbutton.bookmark-item[dragover="true"][open="true"] {
   /* currently, the identity box is always LTR */
   -moz-outline-radius-topleft: 2px;
   -moz-outline-radius-bottomleft: 2px;
+  -moz-user-focus: normal;
+}
+
+#identity-box:focus {
+  outline: 1px dotted -moz-DialogText;
 }
 
 #identity-box:hover {
diff --git a/toolkit/content/widgets/autocomplete.xml b/toolkit/content/widgets/autocomplete.xml
index f9e9c125cf44..6aca23d4d1b7 100644
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -386,6 +386,9 @@
       <method name="onKeyPress">
         <parameter name="aEvent"/>
         <body><![CDATA[
+          if (aEvent.target.localName != "textbox")
+            return;  // Let child buttons of autocomplete take input
+
           //XXXpch this is so bogus...
           if (aEvent.getPreventDefault())
             return false;
@@ -536,7 +539,7 @@
     </resources>
 
     <content ignorekeys="true">
-      <xul:tree anonid="tree" class="autocomplete-tree plain" hidecolumnpicker="true" flex="1">
+      <xul:tree anonid="tree" class="autocomplete-tree plain" hidecolumnpicker="true" flex="1" seltype="single">
         <xul:treecols anonid="treecols">
           <xul:treecol id="treecolAutoCompleteValue" class="autocomplete-treecol" flex="1" overflow="true"/>
         </xul:treecols>
@@ -741,6 +744,10 @@
 
   <binding id="autocomplete-base-popup" extends="chrome://global/content/bindings/popup.xml#popup">
     <implementation implements="nsIAutoCompletePopup">
+      <!-- nsIAccessible from #popup -->
+      <property name="accessibleType" readonly="true"
+                onget="return Components.interfaces.nsIAccessibleProvider.NoAccessible;"/>
+
       <field name="mInput">null</field>
       <field name="mPopupOpen">false</field>
 
@@ -835,6 +842,9 @@
       ]]></handler>
 
       <handler event="popuphiding"><![CDATA[
+        var isListActive = true;
+        if (this.selectedIndex == -1)
+          isListActive = false;
         var controller = this.view.QueryInterface(Components.interfaces.nsIAutoCompleteController);
         controller.stopSearch();
 
@@ -850,6 +860,10 @@
         // when the popupshowing handler runs.
         this.mInput.maxRows = this._normalMaxRows;
         this._normalMaxRows = -1;
+        // If the list was being navigated and then closed, make sure
+        // we fire accessible focus event back to textbox
+        if (isListActive)
+          this.mInput._focus();
       ]]></handler>
     </handlers>
   </binding>
diff --git a/toolkit/content/widgets/popup.xml b/toolkit/content/widgets/popup.xml
index 4fe877f15620..dc58ce82ed03 100644
--- a/toolkit/content/widgets/popup.xml
+++ b/toolkit/content/widgets/popup.xml
@@ -9,20 +9,8 @@
     <resources>
       <stylesheet src="chrome://global/skin/popup.css"/>
     </resources>
-  </binding>
-
-  <binding id="panel"
-           extends="chrome://global/content/bindings/popup.xml#popup-base">
-
-    <implementation implements="nsIDOMXULPopupElement, nsIAccessibleProvider">
-      <property name="accessibleType" readonly="true">
-        <getter>
-          <![CDATA[
-            return Components.interfaces.nsIAccessibleProvider.XULMenupopup;
-          ]]>
-        </getter>
-      </property>
 
+    <implementation implements="nsIDOMXULPopupElement">
       <property name="position" onget="return this.getAttribute('position');"
                                 onset="this.setAttribute('position', val); return val;"/>
       <property name="popupBoxObject">
@@ -167,13 +155,24 @@
   </binding>
 
   <binding id="popup"
-           extends="chrome://global/content/bindings/popup.xml#panel">
+           extends="chrome://global/content/bindings/popup.xml#popup-base">
+    
     <content>
       <xul:arrowscrollbox class="popup-internal-box" flex="1" orient="vertical">
         <children/>
       </xul:arrowscrollbox>
     </content>
 
+    <implementation implements="nsIAccessibleProvider">
+      <property name="accessibleType" readonly="true">
+        <getter>
+          <![CDATA[
+              return Components.interfaces.nsIAccessibleProvider.XULMenupopup;
+            ]]>
+        </getter>
+      </property>
+    </implementation>
+
     <handlers>
       <handler event="contextmenu" action="event.preventDefault();"/>
 
@@ -198,6 +197,64 @@
     </handlers>
   </binding>
 
+  <binding id="panel"
+           extends="chrome://global/content/bindings/popup.xml#popup-base">
+    <!-- This separate binding for dialog-like panels - not menu, list or autocomplete popups
+         exposes the popup as an alert or a pane, depending on whether it is always intended
+         to get keyboard navigation when it opens -->
+    <implementation implements="nsIDOMXULPopupElement, nsIAccessibleProvider">
+      <property name="accessibleType" readonly="true">
+        <getter>
+          <![CDATA[
+          return (this.getAttribute("noautofocus") == "true") ?
+                       Components.interfaces.nsIAccessibleProvider.XULAlert :
+                       Components.interfaces.nsIAccessibleProvider.XULPane;
+        ]]></getter>
+      </property>
+      <field name="_prevFocus">0</field>
+    </implementation>
+    
+    <handlers>
+      <handler event="popupshowing"><![CDATA[
+        // Capture the previous focus before has a chance to get set inside the panel
+        try {
+          this._prevFocus = document.commandDispatcher.focusedElement;
+          if (!this._prevFocus)  // Content window has focus
+            this._prevFocus = document.commandDispatcher.focusedWindow;
+        } catch (ex) {}
+       ]]></handler>
+      <handler event="popupshown"><![CDATA[
+        // Fire event for accessibility APIs
+        var alertEvent = document.createEvent("Events");
+        alertEvent.initEvent("AlertActive", true, true);
+        this.dispatchEvent(alertEvent);
+       ]]></handler>
+      <handler event="popuphiding"><![CDATA[
+        function restoreFocusIfInPanel(aPanel, currentFocus, prevFocus) {
+          try {
+            if (document.commandDispatcher.focusedWindow != window)
+              return; // Focus has already been set to a window outside of this panel
+          } catch(ex) {}
+          var ancestorOfFocus = currentFocus;
+          while (ancestorOfFocus) {
+            if (ancestorOfFocus == aPanel) {
+              // Focus was set on an element inside this panel,
+              // so we need to move it back to where it was previously
+              prevFocus.focus();
+              return;
+            }
+            ancestorOfFocus = ancestorOfFocus.parentNode;
+          }
+        }
+        try {
+          if (this._prevFocus && this.getAttribute("norestorefocus") != "true")
+            setTimeout(restoreFocusIfInPanel, 0, this, document.commandDispatcher.focusedElement,
+                       this._prevFocus);
+        } catch(ex) { }
+      ]]></handler>
+    </handlers>
+  </binding>
+
   <binding id="tooltip" extends="chrome://global/content/bindings/popup.xml#popup">
     <content>
       <children>
diff --git a/toolkit/content/widgets/textbox.xml b/toolkit/content/widgets/textbox.xml
index 1546249c52e2..f7f5d78266ee 100644
--- a/toolkit/content/widgets/textbox.xml
+++ b/toolkit/content/widgets/textbox.xml
@@ -157,8 +157,10 @@
       <handler event="focus" phase="capturing">
         <![CDATA[
           if (!this.hasAttribute("focused")) {
-            if (event.originalTarget != this.inputField)
-              this.inputField.focus();
+            if (event.originalTarget == this)
+              this.inputField.focus(); // Forward focus to actual HTML input
+            else if (event.originalTarget != this.inputField)
+              return; // Allow other children (e.g. URL bar buttons) to get focus
             else if (this.mIgnoreFocus)
               this.mIgnoreFocus = false;
             else if (this.clickSelectsAll)