Bug 490761 - do not fail when nsIAccessibleValue accessible can't get value from markup, r=marcoz, davidb
authorAlexander Surkov <surkov.alexander@gmail.com>
Wed, 06 May 2009 11:16:36 +0800
changeset 28024 ea3a2176d963eb20033d921c177008272a437b7e
parent 28023 251caf79eba316c11d097fed36c8dad9d4502554
child 28025 71d2d6ae64f5559101640b61334e7e21b8ade5ed
push id6844
push usersurkov.alexander@gmail.com
push dateWed, 06 May 2009 02:17:14 +0000
treeherdermozilla-central@ea3a2176d963 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarcoz, davidb
bugs490761
milestone1.9.2a1pre
Bug 490761 - do not fail when nsIAccessibleValue accessible can't get value from markup, r=marcoz, davidb
accessible/src/base/nsAccessible.cpp
accessible/src/xul/nsXULFormControlAccessible.cpp
accessible/src/xul/nsXULSliderAccessible.cpp
accessible/src/xul/nsXULSliderAccessible.h
accessible/tests/mochitest/Makefile.in
accessible/tests/mochitest/common.js
accessible/tests/mochitest/test_elm_prgrsmtr.xul
accessible/tests/mochitest/test_value.xul
accessible/tests/mochitest/value.js
--- a/accessible/src/base/nsAccessible.cpp
+++ b/accessible/src/base/nsAccessible.cpp
@@ -3218,31 +3218,38 @@ PRBool nsAccessible::CheckVisibilityInPa
 }
 
 nsresult
 nsAccessible::GetAttrValue(nsIAtom *aProperty, double *aValue)
 {
   NS_ENSURE_ARG_POINTER(aValue);
   *aValue = 0;
 
-  if (!mDOMNode)
+  if (IsDefunct())
     return NS_ERROR_FAILURE;  // Node already shut down
 
  if (!mRoleMapEntry || mRoleMapEntry->valueRule == eNoValue)
     return NS_OK_NO_ARIA_VALUE;
 
   nsCOMPtr<nsIContent> content(do_QueryInterface(mDOMNode));
   NS_ENSURE_STATE(content);
 
-  PRInt32 result = NS_OK;
-  nsAutoString value;
-  if (content->GetAttr(kNameSpaceID_None, aProperty, value))
-    *aValue = value.ToFloat(&result);
-
-  return result;
+  nsAutoString attrValue;
+  content->GetAttr(kNameSpaceID_None, aProperty, attrValue);
+
+  // Return zero value if there is no attribute or its value is empty.
+  if (attrValue.IsEmpty())
+    return NS_OK;
+
+  PRInt32 error = NS_OK;
+  double value = attrValue.ToFloat(&error);
+  if (NS_SUCCEEDED(error))
+    *aValue = value;
+
+  return NS_OK;
 }
 
 PRUint32
 nsAccessible::GetActionRule(PRUint32 aStates)
 {
   if (aStates & nsIAccessibleStates::STATE_UNAVAILABLE)
     return eNoAction;
 
--- a/accessible/src/xul/nsXULFormControlAccessible.cpp
+++ b/accessible/src/xul/nsXULFormControlAccessible.cpp
@@ -574,26 +574,35 @@ NS_IMETHODIMP
 nsXULProgressMeterAccessible::GetCurrentValue(double *aCurrentValue)
 {
   nsresult rv = nsFormControlAccessible::GetCurrentValue(aCurrentValue);
   if (rv != NS_OK_NO_ARIA_VALUE)
     return rv;
 
   nsCOMPtr<nsIContent> content(do_QueryInterface(mDOMNode));
 
-  nsAutoString currentValue;
-  content->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::value, currentValue);
+  nsAutoString attrValue;
+  content->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::value, attrValue);
+
+  // Return zero value if there is no attribute or its value is empty.
+  if (attrValue.IsEmpty())
+    return NS_OK;
 
-  PRInt32 result = NS_OK;
-  if (content->HasAttr(kNameSpaceID_None, nsAccessibilityAtoms::max))
-    *aCurrentValue = currentValue.ToFloat(&result);
-  else
-    *aCurrentValue = currentValue.ToFloat(&result) / 100;
+  PRInt32 error = NS_OK;
+  double value = attrValue.ToFloat(&error);
+  if (NS_FAILED(error))
+    return NS_OK; // Zero value because of wrong markup.
 
-  return result;
+  // If no max value then value is between 0 and 1 (refer to GetMaximumValue()
+  // method where max value is assumed to be equal to 1 in this case).
+  if (!content->HasAttr(kNameSpaceID_None, nsAccessibilityAtoms::max))
+    value /= 100;
+
+  *aCurrentValue = value;
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 nsXULProgressMeterAccessible::SetCurrentValue(double aValue)
 {
   return NS_ERROR_FAILURE; // Progress meters are readonly!
 }
 
--- a/accessible/src/xul/nsXULSliderAccessible.cpp
+++ b/accessible/src/xul/nsXULSliderAccessible.cpp
@@ -44,16 +44,22 @@
 // nsXULSliderAccessible
 
 nsXULSliderAccessible::nsXULSliderAccessible(nsIDOMNode* aNode,
                                              nsIWeakReference* aShell) :
   nsAccessibleWrap(aNode, aShell)
 {
 }
 
+// nsISupports
+
+NS_IMPL_ISUPPORTS_INHERITED1(nsXULSliderAccessible,
+                             nsAccessibleWrap,
+                             nsIAccessibleValue)
+
 // nsIAccessible
 
 nsresult
 nsXULSliderAccessible::GetRoleInternal(PRUint32 *aRole)
 {
   *aRole = nsIAccessibleRole::ROLE_SLIDER;
   return NS_OK;
 }
@@ -170,52 +176,59 @@ nsXULSliderAccessible::GetSliderNode()
   return NS_FAILED(rv) ? nsnull : sliderNode;
 }
 
 nsresult
 nsXULSliderAccessible::GetSliderAttr(nsIAtom *aName, nsAString& aValue)
 {
   aValue.Truncate();
 
-  if (!mDOMNode)
+  if (IsDefunct())
     return NS_ERROR_FAILURE;
 
   nsCOMPtr<nsIContent> sliderNode(GetSliderNode());
   NS_ENSURE_STATE(sliderNode);
 
   sliderNode->GetAttr(kNameSpaceID_None, aName, aValue);
   return NS_OK;
 }
 
 nsresult
 nsXULSliderAccessible::SetSliderAttr(nsIAtom *aName, const nsAString& aValue)
 {
-  if (!mDOMNode)
+  if (IsDefunct())
     return NS_ERROR_FAILURE;
 
   nsCOMPtr<nsIContent> sliderNode(GetSliderNode());
   NS_ENSURE_STATE(sliderNode);
 
   sliderNode->SetAttr(kNameSpaceID_None, aName, aValue, PR_TRUE);
   return NS_OK;
 }
 
 nsresult
 nsXULSliderAccessible::GetSliderAttr(nsIAtom *aName, double *aValue)
 {
   NS_ENSURE_ARG_POINTER(aValue);
   *aValue = 0;
 
-  nsAutoString value;
-  nsresult rv = GetSliderAttr(aName, value);
+  nsAutoString attrValue;
+  nsresult rv = GetSliderAttr(aName, attrValue);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  // Return zero value if there is no attribute or its value is empty.
+  if (attrValue.IsEmpty())
+    return NS_OK;
+
   PRInt32 error = NS_OK;
-  *aValue = value.ToFloat(&error);
-  return error;
+  double value = attrValue.ToFloat(&error);
+  if (NS_SUCCEEDED(error))
+    *aValue = value;
+
+  return NS_OK;
 }
 
 nsresult
 nsXULSliderAccessible::SetSliderAttr(nsIAtom *aName, double aValue)
 {
   nsAutoString value;
   value.AppendFloat(aValue);
 
--- a/accessible/src/xul/nsXULSliderAccessible.h
+++ b/accessible/src/xul/nsXULSliderAccessible.h
@@ -43,25 +43,24 @@
 
 #include "nsIDOMElement.h"
 
 class nsXULSliderAccessible : public nsAccessibleWrap
 {
 public:
   nsXULSliderAccessible(nsIDOMNode* aNode, nsIWeakReference* aShell);
 
+  // nsISupports
+  NS_DECL_ISUPPORTS_INHERITED
+
   // nsIAccessible
   NS_IMETHOD GetValue(nsAString& aValue);
 
   // nsIAccessibleValue
-  NS_IMETHOD GetMaximumValue(double *aMaximumValue);
-  NS_IMETHOD GetMinimumValue(double *aMinimumValue);
-  NS_IMETHOD GetMinimumIncrement(double *aMinIncrement);
-  NS_IMETHOD GetCurrentValue(double *aValue);
-  NS_IMETHOD SetCurrentValue(double aValue);
+  NS_DECL_NSIACCESSIBLEVALUE
 
   // nsPIAccessible
   NS_IMETHOD GetAllowsAnonChildAccessibles(PRBool *aAllowsAnonChildren);
 
   // nsAccessible
   virtual nsresult GetRoleInternal(PRUint32 *aRole);
 
 protected:
--- a/accessible/tests/mochitest/Makefile.in
+++ b/accessible/tests/mochitest/Makefile.in
@@ -59,31 +59,31 @@ include $(topsrcdir)/config/rules.mk
 		nsIAccessible_name.css \
 		nsIAccessible_name.js \
 		nsIAccessible_name.xbl \
  		nsIAccessible_selects.js \
 		nsIAccessible_states.js \
 		nsIAccessibleEditableText.js \
 		relations.js \
 		role.js \
+		value.js \
 		test_accessnode_invalidation.html \
 		test_actions_aria.html \
 	$(warning test_actions_inputs.html temporarily disabled) \
 		test_actions.xul \
 		test_aria_activedescendant.html \
 		test_aria_role_article.html \
 		test_aria_role_equation.html \
 		test_aria_token_attrs.html \
 		test_bug420863.html \
 	$(warning	test_childAtPoint.xul temporarily disabled) \
 		test_cssattrs.html \
 		test_descr.html \
 		test_elm_filectrl.html \
 		test_elm_media.html \
-		test_elm_prgrsmtr.xul \
 		test_elm_txtcntnr.html \
 		test_events_caretmove.html \
 		test_events_mutation.html \
 		test_events_tree.xul \
 		test_groupattrs.xul \
 		test_groupattrs.html \
 		test_name_markup.html \
 	$(warning test_table_indexes.html temporarily disabled) \
@@ -116,16 +116,17 @@ include $(topsrcdir)/config/rules.mk
 		test_states_frames.html \
 		test_table_1.html \
 		test_table_2.html \
 		test_table_3.html \
 		test_table_4.html \
 		test_textattrs.html \
 		test_textboxes.html \
 		test_textboxes.xul \
+		test_value.xul \
 		testTextboxes.js \
 		treeview.js \
 		z_states_frame.html \
 		z_states_framearticle.html \
 		z_states_framecheckbox.html \
 		z_states_frametextbox.html \
 		$(NULL)
 
--- a/accessible/tests/mochitest/common.js
+++ b/accessible/tests/mochitest/common.js
@@ -210,28 +210,28 @@ function getAccessible(aAccOrElmOrID, aI
     return acc;
 
   if (aInterfaces instanceof Array) {
     for (var index = 0; index < aInterfaces.length; index++) {
       try {
         acc.QueryInterface(aInterfaces[index]);
       } catch (e) {
         if (!(aDoNotFailIf & DONOTFAIL_IF_NO_INTERFACE))
-          ok(false, "Can't query " + aInterfaces[index] + " for " + aID);
+          ok(false, "Can't query " + aInterfaces[index] + " for " + aAccOrElmOrID);
 
         return null;
       }
     }
     return acc;
   }
   
   try {
     acc.QueryInterface(aInterfaces);
   } catch (e) {
-    ok(false, "Can't query " + aInterfaces + " for " + aID);
+    ok(false, "Can't query " + aInterfaces + " for " + aAccOrElmOrID);
     return null;
   }
   
   return acc;
 }
 
 /**
  * Return true if the given identifier has an accessible, or exposes the wanted
rename from accessible/tests/mochitest/test_elm_prgrsmtr.xul
rename to accessible/tests/mochitest/test_value.xul
--- a/accessible/tests/mochitest/test_elm_prgrsmtr.xul
+++ b/accessible/tests/mochitest/test_value.xul
@@ -8,45 +8,35 @@
 
   <script type="application/javascript" 
           src="chrome://mochikit/content/MochiKit/packed.js" />
   <script type="application/javascript"
           src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js" />
 
   <script type="application/javascript"
           src="chrome://mochikit/content/a11y/accessible/common.js" />
+  <script type="application/javascript"
+          src="chrome://mochikit/content/a11y/accessible/value.js" />
 
   <script type="application/javascript">
   <![CDATA[
-    /**
-     * Helper function to test nsIAccessibleValue interface.
-     */
-    function testValue(aAccOrElmOrId, aValue, aCurrValue,
-                       aMinValue, aMaxValue, aMinIncr)
-    {
-      var acc = getAccessible(aAccOrElmOrId, [nsIAccessibleValue]);
-      if (!acc)
-        return;
-
-      is(acc.value, aValue, "Wrong value of " + prettyName(aAccOrElmOrId));
-
-      is(acc.currentValue, aCurrValue,
-         "Wrong current value of " + prettyName(aAccOrElmOrId));
-      is(acc.minimumValue, aMinValue,
-         "Wrong minimum value of " + prettyName(aAccOrElmOrId));
-      is(acc.maximumValue, aMaxValue,
-         "Wrong maximum value of " + prettyName(aAccOrElmOrId));
-      is(acc.minimumIncrement, aMinIncr,
-         "Wrong minimum increment value of " + prettyName(aAccOrElmOrId));
-    }
-
     function doTest()
     {
+      // progressmeter
       testValue("pm1", "50%", .5, 0, 1, 0);
       testValue("pm2", "50%", 500, 0, 1000, 0);
+      testValue("pm3", "0%", 0, 0, 1, 0);
+
+      // scale
+      testValue("sc1", "500", 500, 0, 1000, 10);
+      testValue("sc2", "", 0, 0, 0, 0);
+
+      // aria progressbar
+      testValue("ariapb1", "500", 500, 0, 1000, 0);
+      testValue("ariapb2", "", 0, 0, 0, 0);
 
       SimpleTest.finish();
     }
 
     SimpleTest.waitForExplicitFinish();
     addA11yLoadEvent(doTest);
   ]]>
   </script>
@@ -60,14 +50,25 @@
       </a><br/>
       <p id="display"></p>
       <div id="content" style="display: none">
       </div>
       <pre id="test">
       </pre>
     </body>
 
+    <!-- progressmeter -->
     <progressmeter id="pm1" value="50"/>
     <progressmeter id="pm2" value="500" max="1000"/>
+    <progressmeter id="pm3"/>
+
+    <!-- scale -->
+    <scale id="sc1" value="500" max="1000" increment="10"/>
+    <scale id="sc2"/>
+
+    <!-- aria -->
+    <description id="ariapb1" role="progressbar"
+                 aria-valuenow="500" aria-valuemin="0" aria-valuemax="1000"/>
+    <description id="ariapb2" role="progressbar"/>
   </hbox>
 
 </window>
 
new file mode 100644
--- /dev/null
+++ b/accessible/tests/mochitest/value.js
@@ -0,0 +1,32 @@
+////////////////////////////////////////////////////////////////////////////////
+// Public methods
+
+/**
+ * Tests nsIAccessibleValue interface.
+ *
+ * @param aAccOrElmOrId  [in] identifier of accessible
+ * @param aValue         [in] accessible value (nsIAccessible::value)
+ * @param aCurrValue     [in] current value (nsIAccessibleValue::currentValue)
+ * @param aMinValue      [in] minimum value (nsIAccessibleValue::minimumValue)
+ * @param aMaxValue      [in] maximumn value (nsIAccessibleValue::maximumValue)
+ * @param aMinIncr       [in] minimum increment value
+ *                        (nsIAccessibleValue::minimumIncrement)
+ */
+function testValue(aAccOrElmOrId, aValue, aCurrValue,
+                   aMinValue, aMaxValue, aMinIncr)
+{
+  var acc = getAccessible(aAccOrElmOrId, [nsIAccessibleValue]);
+  if (!acc)
+    return;
+
+  is(acc.value, aValue, "Wrong value of " + prettyName(aAccOrElmOrId));
+
+  is(acc.currentValue, aCurrValue,
+     "Wrong current value of " + prettyName(aAccOrElmOrId));
+  is(acc.minimumValue, aMinValue,
+     "Wrong minimum value of " + prettyName(aAccOrElmOrId));
+  is(acc.maximumValue, aMaxValue,
+     "Wrong maximum value of " + prettyName(aAccOrElmOrId));
+  is(acc.minimumIncrement, aMinIncr,
+     "Wrong minimum increment value of " + prettyName(aAccOrElmOrId));
+}