150606 - Implementation of nsIAccessibleValue

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Gilbert Fang]

see also bugsceri 34

Comment 1

[Kyle Yuan]

Jessie will be in charge of this bug.

Comment 2

[Gilbert Fang]

See mozilla/ layout/ xul/ base/ src/ nsProgressMeterFrame.cpp 
132     nsAutoString value;
133     mContent->GetAttr(kNameSpaceID_None, nsHTMLAtoms::value, value);
134 
135     PRInt32 error;
136     PRInt32 flex = value.ToInteger(&error);
137     if (flex < 0) flex = 0;
138     if (flex > 100) flex = 100;

Oh, this would mean that ProgressMeter's min/max are sure 0/100. And it really
makes things easy.:-)

Comment 3

[Gilbert Fang]

Extend class nsXULProgressMeterAccessible  to support the  nsIAccessibleValue.
Is there any other xul control which needs to support the  nsIAccessibleValue, I
will file a new bug for that. This one is only for nsXULProgressMeterAccessible.

Comment 4

[Gilbert Fang]

See mozilla/ accessible/ src/ xul/ nsXULFormControlAccessible.cpp 
458 NS_IMETHODIMP nsXULProgressMeterAccessible::GetAccValue(nsAString& _retval)
459 {
460   nsCOMPtr<nsIDOMElement> element(do_QueryInterface(mDOMNode));
461   NS_ASSERTION(element, "No element for DOM node!");
462   element->GetAttribute(NS_LITERAL_STRING("value"), _retval);
463   if (!_retval.IsEmpty() && _retval.Last() != '%')
464     _retval.Append(NS_LITERAL_STRING("%"));
465   return NS_OK;
466 }
aha, it is also easy to get the current value. :-)

Comment 5

[Gilbert Fang]

#include "nsISupports.idl"

[scriptable, uuid(F4ABBC2F-0F28-47DC-A9E9-F7A1719AB2BE)]
interface nsIAccessibleValue : nsISupports
{
  readonly attribute double maximumValue;
  readonly attribute double minimumValue;
  readonly attribute double currentValue;

    /**
      * We want to be able to return a success condition of the value
      *   getting set. ie if the value is not within the interval of
      *   minimumValue-maximumValue
      */
  boolean setCurrentValue (in double value);
};
For progressmeter, I suggest that the maximum is 100% =1. Let the ATs to do the 
translation to the percentage. 
The reason is this interface may be supported by other controls, and we should 
keep the meaning of numbers consistent.

Comment 6

[Gilbert Fang]

If I write the code like this:

  nsCOMPtr<nsIDOMElement> element(do_QueryInterface(mDOMNode));
  NS_ASSERTION(element, "No element for DOM node!");
  nsPrintfCString currentValue("%.0f%",aValue);
  if (NS_SUCCEEDED(element->SetAttribute(NS_LITERAL_STRING("value"), 
currentValue))) {
    *_retval = PR_TRUE;
    return NS_OK;
  }
Then the vc will complain: 
e:\cmtrunksb\mozilla\accessible\src\xul\nsXULFormControlAccessible.cpp(511) : 
error C2664: 'SetAttribute' : cannot convert parameter 2 from 'class 
nsPrintfCString' to 'const class nsAString &'
        Reason: cannot convert from 'class nsPrintfCString' to 'const class 
nsAString'
        No constructor could take the source type, or constructor overload 
resolution was ambiguous
Why nsPrintfCString can not convert automatically, should we file a bug to 
improve it?

Comment 7

[Gilbert Fang]

oh, I should use NS_ConvertUTF8toUCS2 to convert it. Thanks ot beisi and sirLuxalot.

Comment 8

[Gilbert Fang]

Attachment: extending the nsXULProgressMeterAccessible (patch_bz150606_20020704_d)

This patch extends nsXULProgressMeterAccessible to support the 
nsIAccessibleValue.
need r=?

Comment 9

[Aaron Leventhal]

Comment on attachment 90203 [details] [diff] [review]
extending  the nsXULProgressMeterAccessible  (patch_bz150606_20020704_d)

In GetCurrentValue() why are you using a variable called "error"? It doesn't
sound much like an error at all -- perhaps you should call it currentValue or
something?
+  PRInt32 error;
+  *aCurrentValue = currentValue.ToFloat(&error) / 100;

In SetCurrentValue(),  why not consolodate into
double min, max;
+  double min;
+  GetMinimumValue(&min);
+  double max;
+  GetMaximumValue(&max);

r=aaronl

Comment 10

[Gilbert Fang]

Hi, aaronl, thank u for review. 
here is my replay:
1) For ToFloat, 
Please see mozilla/ string/ obsolete/ nsString.cpp 
460 /**
461  * Perform string to float conversion.
462  * @update  gess 01/04/99
463  * @param   aErrorCode will contain error if one occurs
464  * @return  float rep of string value
465  */
466 float nsCString::ToFloat(PRInt32* aErrorCode) 

So the variable error is for that error code. 

2)Okay. That is because by now, I never found in mozilla, there exists that 
defining  two variable in ONE line. I am not sure whether it is a convention. 
If u think they should be in one line, that is all right. 

Comment 11

[Gilbert Fang]

Attachment: new patch after aaronl's review (patch_bz150606_20020705_a)

Extending the nsXULProgressMeterAccessible to support nsIAccessibleValue.
This patch modifys  one line of ( 90203: extending the
nsXULProgressMeterAccessible (patch_bz150606_20020704_d) following the arronls'
review. 
Please super-revies this patch.

Comment 12

[Jessie Li]

Attachment: Test Case for nsIAccessibleValue for XUL element

Comment 13

[Aaron Leventhal]

Comment on attachment 90268 [details] [diff] [review]
new patch after aaronl's review (patch_bz150606_20020705_a)

r=aaronl

Comment 14

[jag (Peter Annema)]

Comment on attachment 90268 [details] [diff] [review]
new patch after aaronl's review (patch_bz150606_20020705_a)

You could use this:

PRUint32 value = PRUint32(aValue * 100.0 + 0.5);
nsAutoString valueString;
valueString.AppendInt(value);

Comment 15

[Gilbert Fang]

Attachment: new patch following jag's sr(patch_bz150606_20020708_b)

need r= and sr= ?

Comment 16

[Kyle Yuan]

Comment on attachment 90578 [details] [diff] [review]
new patch following jag's sr(patch_bz150606_20020708_b)

r=kyle

Comment 17

[jag (Peter Annema)]

Comment on attachment 90578 [details] [diff] [review]
new patch following jag's sr(patch_bz150606_20020708_b)

+  if (aValue >= max && aValue <= min)
+    return NS_ERROR_INVALID_ARG;

So if the min is 0.0 and the max is 1.0 this means that your value can't be 0.0
or 1.0 itself. Is this what you want?

Comment 18

[Gilbert Fang]

oh, my god, jag. you r really cute. 
I do not want to mean that actually. :-(
I think I really should do is 
 if (aValue > max || aValue < min)
    return NS_ERROR_INVALID_ARG;

I am so embarrassed  by this careless mistakes. 
Could u please kindly give me a super-review again? 

Comment 19

[Gilbert Fang]

Attachment: second patch after jag's review(patch_bz150606_20020710_b)

could jag spend some time to see it again? :-)

Comment 20

[jag (Peter Annema)]

Comment on attachment 90723 [details] [diff] [review]
second patch after jag's review(patch_bz150606_20020710_b)

sr=jag, carrying over r=

Comment 21

[Asa Dotzler [:asa]]

Comment on attachment 90723 [details] [diff] [review]
second patch after jag's review(patch_bz150606_20020710_b)

a=asa (on behalf of drivers) for checkin to the 1.1 trunk.

Comment 22

[Pete Zha]

checked in