Bug 468451 - Images with empty alt attribute no longer get an empty accessible name, but return NULL instead, r=aaronlev, marcoz, davidb
authorAlexander Surkov <surkov.alexander@gmail.com>
Wed, 17 Dec 2008 00:13:49 +0800
changeset 22844 160661559c6273788ceda8509fb6046b7993c23f
parent 22843 fc61fc41c4b4fc4b289e3fc3f5c64f1d764e361a
child 22845 d637f84ee780afbeb5f52be5847ce0a9da18e676
push idunknown
push userunknown
push dateunknown
reviewersaaronlev, marcoz, davidb
bugs468451
milestone1.9.2a1pre
Bug 468451 - Images with empty alt attribute no longer get an empty accessible name, but return NULL instead, r=aaronlev, marcoz, davidb
accessible/public/nsIAccessible.idl
accessible/src/base/nsAccessible.cpp
accessible/src/base/nsAccessible.h
accessible/src/html/nsHTMLImageAccessible.cpp
accessible/tests/mochitest/test_nsIAccessibleImage.html
--- a/accessible/public/nsIAccessible.idl
+++ b/accessible/public/nsIAccessible.idl
@@ -98,17 +98,24 @@ interface nsIAccessible : nsISupports
 
   /**
    * The 0-based index of this accessible in its parent's list of children,
    * or -1 if this accessible does not have a parent.
    */
   readonly attribute long indexInParent;
 
   /**
-   * Accessible name -- the main text equivalent for this node
+   * Accessible name -- the main text equivalent for this node. The name is
+   * specified by ARIA or by native markup. Example of ARIA markup is
+   * aria-labelledby attribute placed on element of this accessible. Example
+   * of native markup is HTML label linked with HTML element of this accessible.
+   *
+   * Value can be string or null. A null value indicates that AT may attempt to
+   * compute the name. Any string value, including the empty string, should be
+   * considered author-intentional, and respected.
    */
   attribute AString name;
 
   /**
    * Accessible value -- a number or a secondary text equivalent for this node
    * Widgets that use role attribute can force a value using the valuenow attribute
    */
   readonly attribute AString value;
--- a/accessible/src/base/nsAccessible.cpp
+++ b/accessible/src/base/nsAccessible.cpp
@@ -303,17 +303,17 @@ nsAccessible::GetName(nsAString& aName)
   else
     return NS_OK;
 
   // XXX: if CompressWhiteSpace worked on nsAString we could avoid a copy.
   nsAutoString name;
   if (content->GetAttr(kNameSpaceID_None, tooltipAttr, name)) {
     name.CompressWhitespace();
     aName = name;
-  } else {
+  } else if (rv != NS_OK_EMPTY_NAME) {
     aName.SetIsVoid(PR_TRUE);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP nsAccessible::GetDescription(nsAString& aDescription)
 {
--- a/accessible/src/base/nsAccessible.h
+++ b/accessible/src/base/nsAccessible.h
@@ -61,19 +61,24 @@
 struct nsRect;
 class nsIContent;
 class nsIFrame;
 class nsIPresShell;
 class nsIDOMNode;
 class nsIAtom;
 class nsIView;
 
+// see nsAccessible::GetAttrValue
 #define NS_OK_NO_ARIA_VALUE \
 NS_ERROR_GENERATE_SUCCESS(NS_ERROR_MODULE_GENERAL, 0x21)
 
+// see nsAccessible::GetNameInternal
+#define NS_OK_EMPTY_NAME \
+NS_ERROR_GENERATE_SUCCESS(NS_ERROR_MODULE_GENERAL, 0x23)
+
 // Saves a data member -- if child count equals this value we haven't
 // cached children or child count yet
 enum { eChildCountUninitialized = -1 };
 
 class nsAccessibleDOMStringList : public nsIDOMDOMStringList
 {
 public:
   nsAccessibleDOMStringList();
@@ -130,17 +135,23 @@ public:
 
   /**
    * Returns the accessible name specified by ARIA.
    */
   nsresult GetARIAName(nsAString& aName);
 
   /**
    * Returns the accessible name provided by native markup. It doesn't take
-   * into account ARIA stuffs used to specify the name.
+   * into account ARIA markup used to specify the name.
+   *
+   * @param  aName             [out] the accessible name
+   *
+   * @return NS_OK_EMPTY_NAME  points empty name was specified by native markup
+   *                           explicitly (see nsIAccessible::name attribute for
+   *                           details)
    */
   virtual nsresult GetNameInternal(nsAString& aName);
 
   /**
    * Return the state of accessible that doesn't take into account ARIA states.
    * Use nsIAccessible::state to get all states for accessible. If
    * second argument is omitted then second bit field of accessible state won't
    * be calculated.
--- a/accessible/src/html/nsHTMLImageAccessible.cpp
+++ b/accessible/src/html/nsHTMLImageAccessible.cpp
@@ -122,33 +122,31 @@ nsHTMLImageAccessible::GetStateInternal(
   }
 
   return NS_OK;
 }
 
 nsresult
 nsHTMLImageAccessible::GetNameInternal(nsAString& aName)
 {
-  // No alt attribute means AT can repair if there is no accessible name
-  // alt="" with no title or aria-labelledby means image is presentational and 
-  // AT should leave accessible name empty
-
   nsCOMPtr<nsIContent> content(do_QueryInterface(mDOMNode));
   PRBool hasAltAttrib =
     content->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::alt, aName);
   if (!aName.IsEmpty())
     return NS_OK;
 
   nsresult rv = nsAccessible::GetNameInternal(aName);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  if (aName.IsVoid() && hasAltAttrib) {
-    // No accessible name but empty alt attribute is present. This means a name
-    // was provided by author and AT repair of the name isn't allowed.
-    aName.Truncate();
+  if (aName.IsEmpty() && hasAltAttrib) {
+    // No accessible name but empty 'alt' attribute is present. If further name
+    // computation algorithm doesn't provide non empty name then it means
+    // an empty 'alt' attribute was used to indicate a decorative image (see
+    // nsIAccessible::name attribute for details).
+    return NS_OK_EMPTY_NAME;
   }
 
   return NS_OK;
 }
 
 /* wstring getRole (); */
 NS_IMETHODIMP nsHTMLImageAccessible::GetRole(PRUint32 *_retval)
 {
--- a/accessible/tests/mochitest/test_nsIAccessibleImage.html
+++ b/accessible/tests/mochitest/test_nsIAccessibleImage.html
@@ -142,20 +142,20 @@ https://bugzilla.mozilla.org/show_bug.cg
 
       // Test non-linked image with title attribute
       testThis("nonLinkedImageWithTitle", "MoFo logo", "moz.png", 89, 38);
 
       // Test linked image with title attribute
       testThis("linkedImageWithTitle", "Link to MoFo", "moz.png", 93, 42);
 
       // Test simple image with empty alt attribute
-//      testThis("nonLinkedImageEmptyAlt", "", "moz.png", 89, 38);
+      testThis("nonLinkedImageEmptyAlt", "", "moz.png", 89, 38);
 
       // Test linked image with empty alt attribute
-//      testThis("linkedImageEmptyAlt", "", "moz.png", 93, 42);
+      testThis("linkedImageEmptyAlt", "", "moz.png", 93, 42);
 
       // Test simple image with empty alt attribute and title
       testThis("nonLinkedImageEmptyAltAndTitle", "MozillaFoundation", "moz.png", 89, 38);
 
       // Test linked image with empty alt attribute and title
       testThis("linkedImageEmptyAltAndTitle", "Link to Mozilla Foundation", "moz.png", 93, 42);
 
       // Image with long desc