Bug 583533 - Implement AccessKeyLabel attribute r=smaug, ui-review=faaborg

content/base/public/nsContentUtils.h

@@ -1664,6 +1664,12 @@ public:
    */
   static PRBool IsFocusedContent(const nsIContent *aContent);
 
+  static void GetShiftText(nsAString& text);
+  static void GetControlText(nsAString& text);
+  static void GetMetaText(nsAString& text);
+  static void GetAltText(nsAString& text);
+  static void GetModifierSeparatorText(nsAString& text);
+
   /**
    * Returns if aContent has a tabbable subdocument.
    * A sub document isn't tabbable when it's a zombie document.
@@ -1750,6 +1756,8 @@ private:
                              nsIXPConnectJSObjectHolder** aHolder,
                              PRBool aAllowWrapping);
 
+  static void InitializeModifierStrings();
+
   static nsIDOMScriptObjectFactory *sDOMScriptObjectFactory;
 
   static nsIXPConnect *sXPConnect;
@@ -1812,6 +1820,12 @@ private:
 
   static PRBool sIsHandlingKeyBoardEvent;
   static PRBool sAllowXULXBL_for_file;
+
+  static nsString* sShiftText;
+  static nsString* sControlText;
+  static nsString* sMetaText;
+  static nsString* sAltText;
+  static nsString* sModifierSeparator;
 };
 
 #define NS_HOLD_JS_OBJECTS(obj, clazz)                                         \

content/base/src/nsContentUtils.cpp

@@ -256,6 +256,12 @@ nsIInterfaceRequestor* nsContentUtils::sSameOriginChecker = nsnull;
 PRBool nsContentUtils::sIsHandlingKeyBoardEvent = PR_FALSE;
 PRBool nsContentUtils::sAllowXULXBL_for_file = PR_FALSE;
 
+nsString* nsContentUtils::sShiftText = nsnull;
+nsString* nsContentUtils::sControlText = nsnull;
+nsString* nsContentUtils::sMetaText = nsnull;
+nsString* nsContentUtils::sAltText = nsnull;
+nsString* nsContentUtils::sModifierSeparator = nsnull;
+
 PRBool nsContentUtils::sInitialized = PR_FALSE;
 
 static PLDHashTable sEventListenerManagersHash;
@@ -382,6 +388,81 @@ nsContentUtils::Init()
   return NS_OK;
 }
 
+void
+nsContentUtils::GetShiftText(nsAString& text)
+{
+  if (!sShiftText)
+    InitializeModifierStrings();
+  text.Assign(*sShiftText);
+}
+
+void
+nsContentUtils::GetControlText(nsAString& text)
+{
+  if (!sControlText)
+    InitializeModifierStrings();
+  text.Assign(*sControlText);
+}
+
+void
+nsContentUtils::GetMetaText(nsAString& text)
+{
+  if (!sMetaText)
+    InitializeModifierStrings();
+  text.Assign(*sMetaText);
+}
+
+void
+nsContentUtils::GetAltText(nsAString& text)
+{
+  if (!sAltText)
+    InitializeModifierStrings();
+  text.Assign(*sAltText);
+}
+
+void
+nsContentUtils::GetModifierSeparatorText(nsAString& text)
+{
+  if (!sModifierSeparator)
+    InitializeModifierStrings();
+  text.Assign(*sModifierSeparator);
+}
+
+void
+nsContentUtils::InitializeModifierStrings()
+{
+  //load the display strings for the keyboard accelerators
+  nsCOMPtr<nsIStringBundleService> bundleService =
+    mozilla::services::GetStringBundleService();
+  nsCOMPtr<nsIStringBundle> bundle;
+  nsresult rv = NS_OK;
+  if (bundleService) {
+    rv = bundleService->CreateBundle( "chrome://global-platform/locale/platformKeys.properties",
+                                      getter_AddRefs(bundle));
+  }
+  
+  NS_ASSERTION(NS_SUCCEEDED(rv) && bundle, "chrome://global/locale/platformKeys.properties could not be loaded");
+  nsXPIDLString shiftModifier;
+  nsXPIDLString metaModifier;
+  nsXPIDLString altModifier;
+  nsXPIDLString controlModifier;
+  nsXPIDLString modifierSeparator;
+  if (bundle) {
+    //macs use symbols for each modifier key, so fetch each from the bundle, which also covers i18n
+    bundle->GetStringFromName(NS_LITERAL_STRING("VK_SHIFT").get(), getter_Copies(shiftModifier));
+    bundle->GetStringFromName(NS_LITERAL_STRING("VK_META").get(), getter_Copies(metaModifier));
+    bundle->GetStringFromName(NS_LITERAL_STRING("VK_ALT").get(), getter_Copies(altModifier));
+    bundle->GetStringFromName(NS_LITERAL_STRING("VK_CONTROL").get(), getter_Copies(controlModifier));
+    bundle->GetStringFromName(NS_LITERAL_STRING("MODIFIER_SEPARATOR").get(), getter_Copies(modifierSeparator));
+  }
+  //if any of these don't exist, we get  an empty string
+  sShiftText = new nsString(shiftModifier);
+  sMetaText = new nsString(metaModifier);
+  sAltText = new nsString(altModifier);
+  sControlText = new nsString(controlModifier);
+  sModifierSeparator = new nsString(modifierSeparator);  
+}
+
 bool nsContentUtils::sImgLoaderInitialized;
 
 void
@@ -1146,6 +1227,17 @@ nsContentUtils::Shutdown()
   delete sBlockedScriptRunners;
   sBlockedScriptRunners = nsnull;
 
+  delete sShiftText;
+  sShiftText = nsnull;
+  delete sControlText;  
+  sControlText = nsnull;
+  delete sMetaText;  
+  sMetaText = nsnull;
+  delete sAltText;  
+  sAltText = nsnull;
+  delete sModifierSeparator;
+  sModifierSeparator = nsnull;
+
   NS_IF_RELEASE(sSameOriginChecker);
   
   nsTextEditorState::ShutDown();

content/events/src/nsEventStateManager.cpp

@@ -1508,6 +1508,35 @@ nsEventStateManager::ExecuteAccessKey(nsTArray<PRUint32>& aAccessCharCodes,
   return PR_FALSE;
 }
 
+PRBool
+nsEventStateManager::GetAccessKeyLabelPrefix(nsAString& aPrefix)
+{
+  aPrefix.Truncate();
+  nsAutoString separator, modifierText;
+  nsContentUtils::GetModifierSeparatorText(separator);
+
+  nsCOMPtr<nsISupports> container = mPresContext->GetContainer();
+  PRInt32 modifier = GetAccessModifierMask(container);
+
+  if (modifier & NS_MODIFIER_CONTROL) {
+    nsContentUtils::GetControlText(modifierText);
+    aPrefix.Append(modifierText + separator);
+  }
+  if (modifier & NS_MODIFIER_META) {
+    nsContentUtils::GetMetaText(modifierText);
+    aPrefix.Append(modifierText + separator);
+  }
+  if (modifier & NS_MODIFIER_ALT) {
+    nsContentUtils::GetAltText(modifierText);
+    aPrefix.Append(modifierText + separator);
+  }
+  if (modifier & NS_MODIFIER_SHIFT) {
+    nsContentUtils::GetShiftText(modifierText);
+    aPrefix.Append(modifierText + separator);
+  }
+  return !aPrefix.IsEmpty();
+}
+
 void
 nsEventStateManager::HandleAccessKey(nsPresContext* aPresContext,
                                      nsKeyEvent *aEvent,

content/events/src/nsEventStateManager.h

@@ -160,6 +160,8 @@ public:
    */
   PRUint32 GetRegisteredAccessKey(nsIContent* aContent);
 
+  PRBool GetAccessKeyLabelPrefix(nsAString& aPrefix);
+
   nsresult SetCursor(PRInt32 aCursor, imgIContainer* aContainer,
                      PRBool aHaveHotspot, float aHotspotX, float aHotspotY,
                      nsIWidget* aWidget, PRBool aLockCursor); 

content/html/content/src/nsGenericHTMLElement.cpp

@@ -463,6 +463,21 @@ nsGenericHTMLElement::SetClassName(const nsAString& aClassName)
 
 NS_IMPL_STRING_ATTR(nsGenericHTMLElement, AccessKey, accesskey)
 
+NS_IMETHODIMP
+nsGenericHTMLElement::GetAccessKeyLabel(nsAString& aLabel)
+{
+  nsPresContext *presContext = GetPresContext();
+
+  if (presContext &&
+    presContext->EventStateManager()->GetAccessKeyLabelPrefix(aLabel)) {
+      nsAutoString suffix;
+      GetAccessKey(suffix);
+      aLabel.Append(suffix);
+  }
+
+  return NS_OK;
+}
+
 static PRBool
 IsBody(nsIContent *aContent)
 {

content/html/content/src/nsGenericHTMLElement.h

@@ -152,6 +152,7 @@ public:
   NS_IMETHOD SetDraggable(PRBool aDraggable);
   NS_IMETHOD GetAccessKey(nsAString &aAccessKey);
   NS_IMETHOD SetAccessKey(const nsAString& aAccessKey);
+  NS_IMETHOD GetAccessKeyLabel(nsAString& aLabel);
   nsresult GetContentEditable(nsAString& aContentEditable);
   nsresult GetIsContentEditable(PRBool* aContentEditable);
   nsresult SetContentEditable(const nsAString &aContentEditable);
@@ -1471,6 +1472,7 @@ protected:
   NS_SCRIPTABLE NS_IMETHOD SetClassName(const nsAString & aClassName) { return _to SetClassName(aClassName); } \
   NS_SCRIPTABLE NS_IMETHOD GetAccessKey(nsAString & aAccessKey) { return _to GetAccessKey(aAccessKey); } \
   NS_SCRIPTABLE NS_IMETHOD SetAccessKey(const nsAString & aAccessKey) { return _to SetAccessKey(aAccessKey); } \
+  NS_SCRIPTABLE NS_IMETHOD GetAccessKeyLabel(nsAString & aLabel) { return _to GetAccessKeyLabel(aLabel); } \
   NS_SCRIPTABLE NS_IMETHOD Blur(void) { return _to Blur(); }
 
 /**

content/html/content/test/Makefile.in

@@ -278,6 +278,7 @@ _TEST_FILES = \
 		test_bug666200.html \
 		test_bug666666.html \
 		test_bug674558.html \
+		test_bug583533.html \
 		test_restore_from_parser_fragment.html \
 		$(NULL)
 

content/html/content/test/test_bug583533.html

@@ -0,0 +1,76 @@
+<!DOCTYPE HTML>
+<html>
+    <!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=583533
+-->
+    <head>
+        <title>Test for Bug 583514</title>
+        <script type="application/javascript" src="/MochiKit/packed.js"></script>
+        <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+        <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+    </head>
+    <body>
+        <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=583533">Mozilla Bug 583533</a>
+        <p id="display"></p>
+        <div id="content">
+            <div id="e" accesskey="a">
+  </div>
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+    /** Test for Bug 583533 **/
+
+    netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
+
+    var sbs = Components.classes["@mozilla.org/intl/stringbundle;1"].
+    getService(Components.interfaces.nsIStringBundleService);
+    var bundle = sbs.createBundle("chrome://global-platform/locale/platformKeys.properties");
+    
+    var shiftText = bundle.GetStringFromName("VK_SHIFT");
+    var altText = bundle.GetStringFromName("VK_ALT");
+    var controlText = bundle.GetStringFromName("VK_CONTROL");
+    var metaText = bundle.GetStringFromName("VK_META");
+    var separatorText = bundle.GetStringFromName("MODIFIER_SEPARATOR");
+
+    var modifier = Components.classes["@mozilla.org/preferences-service;1"]
+                                      .getService(Components.interfaces.nsIPrefBranch)
+                                      .getIntPref("ui.key.contentAccess");
+                                      
+    var domEvent = Components.interfaces.nsIDOMNSEvent;
+    
+    var isShift;
+    var isAlt;
+    var isControl;
+    var isMeta;
+
+    is(modifier < 16 && modifier >= 0, true, "Modifier in range");
+
+    if (modifier & domEvent.META_MASK)
+        isMeta = true;
+    if (modifier & domEvent.SHIFT_MASK)
+        isShift = true;
+    if (modifier & domEvent.CONTROL_MASK)
+        isControl = true;
+    if (modifier & domEvent.ALT_MASK)
+        isAlt = true;
+
+    var label = "";
+
+    if (isControl)
+        label += controlText + separatorText;
+    if (isMeta)
+        label += metaText + separatorText;
+    if (isAlt)
+        label += altText + separatorText;
+    if (isShift)
+        label += shiftText + separatorText;
+
+    label += document.getElementById("e").accessKey;
+
+    is(label, document.getElementById("e").accessKeyLabel, "JS and C++ agree on accessKeyLabel");
+
+</script>
+</pre>
+</body>
+</html>

dom/interfaces/html/nsIDOMHTMLElement.idl

@@ -51,7 +51,7 @@
  * http://www.whatwg.org/specs/web-apps/current-work/
  */
 
-[scriptable, uuid(164c7ebd-2245-42d2-a96f-2bf2d01c1697)]
+[scriptable, uuid(c6e10ab6-ecf4-48e4-aeaa-37724307dfd5)]
 interface nsIDOMHTMLElement : nsIDOMElement
 {
            attribute DOMString        id;
@@ -61,6 +61,7 @@ interface nsIDOMHTMLElement : nsIDOMElement
            attribute DOMString        className;
 
            attribute DOMString        accessKey;
+  readonly attribute DOMString        accessKeyLabel;
 
            void                       blur();
            void                       focus();

layout/xul/base/src/nsMenuFrame.cpp

@@ -92,12 +92,6 @@ static PRInt32 gEatMouseMove = PR_FALSE;
 
 static NS_DEFINE_IID(kLookAndFeelCID, NS_LOOKANDFEEL_CID);
 
-nsrefcnt nsMenuFrame::gRefCnt = 0;
-nsString *nsMenuFrame::gShiftText = nsnull;
-nsString *nsMenuFrame::gControlText = nsnull;
-nsString *nsMenuFrame::gMetaText = nsnull;
-nsString *nsMenuFrame::gAltText = nsnull;
-nsString *nsMenuFrame::gModifierSeparator = nsnull;
 const PRInt32 kBlinkDelay = 67; // milliseconds
 
 // this class is used for dispatching menu activation events asynchronously.
@@ -300,40 +294,6 @@ nsMenuFrame::Init(nsIContent*      aContent,
 
   InitMenuParent(aParent);
 
-  //load the display strings for the keyboard accelerators, but only once
-  if (gRefCnt++ == 0) {
-    nsCOMPtr<nsIStringBundleService> bundleService =
-      mozilla::services::GetStringBundleService();
-    nsCOMPtr<nsIStringBundle> bundle;
-    if (bundleService) {
-      rv = bundleService->CreateBundle( "chrome://global-platform/locale/platformKeys.properties",
-                                        getter_AddRefs(bundle));
-    }
-    
-    NS_ASSERTION(NS_SUCCEEDED(rv) && bundle, "chrome://global/locale/platformKeys.properties could not be loaded");
-    nsXPIDLString shiftModifier;
-    nsXPIDLString metaModifier;
-    nsXPIDLString altModifier;
-    nsXPIDLString controlModifier;
-    nsXPIDLString modifierSeparator;
-    if (NS_SUCCEEDED(rv) && bundle) {
-      //macs use symbols for each modifier key, so fetch each from the bundle, which also covers i18n
-      rv = bundle->GetStringFromName(NS_LITERAL_STRING("VK_SHIFT").get(), getter_Copies(shiftModifier));
-      rv = bundle->GetStringFromName(NS_LITERAL_STRING("VK_META").get(), getter_Copies(metaModifier));
-      rv = bundle->GetStringFromName(NS_LITERAL_STRING("VK_ALT").get(), getter_Copies(altModifier));
-      rv = bundle->GetStringFromName(NS_LITERAL_STRING("VK_CONTROL").get(), getter_Copies(controlModifier));
-      rv = bundle->GetStringFromName(NS_LITERAL_STRING("MODIFIER_SEPARATOR").get(), getter_Copies(modifierSeparator));
-    } else {
-      rv = NS_ERROR_NOT_AVAILABLE;
-    }
-    //if any of these don't exist, we get  an empty string
-    gShiftText = new nsString(shiftModifier);
-    gMetaText = new nsString(metaModifier);
-    gAltText = new nsString(altModifier);
-    gControlText = new nsString(controlModifier);
-    gModifierSeparator = new nsString(modifierSeparator);    
-  }
-
   BuildAcceleratorText(PR_FALSE);
   nsIReflowCallback* cb = new nsASyncMenuInitialization(this);
   NS_ENSURE_TRUE(cb, NS_ERROR_OUT_OF_MEMORY);
@@ -341,23 +301,6 @@ nsMenuFrame::Init(nsIContent*      aContent,
   return rv;
 }
 
-nsMenuFrame::~nsMenuFrame()
-{
-  // Clean up shared statics
-  if (--gRefCnt == 0) {
-    delete gShiftText;
-    gShiftText = nsnull;
-    delete gControlText;  
-    gControlText = nsnull;
-    delete gMetaText;  
-    gMetaText = nsnull;
-    delete gAltText;  
-    gAltText = nsnull;
-    delete gModifierSeparator;
-    gModifierSeparator = nsnull;
-  }
-}
-
 // The following methods are all overridden to ensure that the menupopup frame
 // is placed in the appropriate list.
 nsFrameList
@@ -1143,35 +1086,48 @@ nsMenuFrame::BuildAcceleratorText(PRBool aNotify)
   char* str = ToNewCString(modifiers);
   char* newStr;
   char* token = nsCRT::strtok(str, ", \t", &newStr);
+
+  nsAutoString shiftText;
+  nsAutoString altText;
+  nsAutoString metaText;
+  nsAutoString controlText;
+  nsAutoString modifierSeparator;
+
+  nsContentUtils::GetShiftText(shiftText);
+  nsContentUtils::GetAltText(altText);
+  nsContentUtils::GetMetaText(metaText);
+  nsContentUtils::GetControlText(controlText);
+  nsContentUtils::GetModifierSeparatorText(modifierSeparator);
+
   while (token) {
       
     if (PL_strcmp(token, "shift") == 0)
-      accelText += *gShiftText;
+      accelText += shiftText;
     else if (PL_strcmp(token, "alt") == 0) 
-      accelText += *gAltText; 
+      accelText += altText; 
     else if (PL_strcmp(token, "meta") == 0) 
-      accelText += *gMetaText; 
+      accelText += metaText; 
     else if (PL_strcmp(token, "control") == 0) 
-      accelText += *gControlText; 
+      accelText += controlText; 
     else if (PL_strcmp(token, "accel") == 0) {
       switch (accelKey)
       {
         case nsIDOMKeyEvent::DOM_VK_META:
-          accelText += *gMetaText;
+          accelText += metaText;
           break;
 
         case nsIDOMKeyEvent::DOM_VK_ALT:
-          accelText += *gAltText;
+          accelText += altText;
           break;
 
         case nsIDOMKeyEvent::DOM_VK_CONTROL:
         default:
-          accelText += *gControlText;
+          accelText += controlText;
           break;
       }
     }
     
-    accelText += *gModifierSeparator;
+    accelText += modifierSeparator;
 
     token = nsCRT::strtok(newStr, ", \t", &newStr);
   }

layout/xul/base/src/nsMenuFrame.h

@@ -253,7 +253,7 @@ protected:
   NS_IMETHOD AttributeChanged(PRInt32 aNameSpaceID,
                               nsIAtom* aAttribute,
                               PRInt32 aModType);
-  virtual ~nsMenuFrame();
+  virtual ~nsMenuFrame() { };
 
   PRBool SizeToPopup(nsBoxLayoutState& aState, nsSize& aSize);
 
@@ -289,14 +289,6 @@ protected:
   nsRefPtr<nsXULMenuCommandEvent> mDelayedMenuCommandEvent;
 
   nsString mGroupName;
-  
-  //we load some display strings from platformKeys.properties only once
-  static nsrefcnt gRefCnt; 
-  static nsString *gShiftText;
-  static nsString *gControlText;
-  static nsString *gMetaText;
-  static nsString *gAltText;
-  static nsString *gModifierSeparator;
 
 }; // class nsMenuFrame