1285643 - Make xpcAccessibleValue work with proxied accessibles

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Comment 1

[Michael Li]

Attachment: Make xpcAccessibleValue work with proxied accessibles. r?tbsaunde

Comment 2

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8769328 [details] [diff] [review]
Make xpcAccessibleValue work with proxied accessibles. r?tbsaunde

> inline Accessible*
> xpcAccessibleValue::Intl()
> {
>   return static_cast<xpcAccessibleGeneric*>(this)->mIntl.AsAccessible();
> }

it might be nice to rewrite things so this function can go away.

> NS_IMETHODIMP
> xpcAccessibleValue::GetMaximumValue(double* aValue)
> {
>   NS_ENSURE_ARG_POINTER(aValue);
>   *aValue = 0;
> 
>   if (Intl()->IsDefunct())
>     return NS_ERROR_FAILURE;
>+  if (IntlGeneric().IsNull())
>+    return NS_ERROR_FAILURE;

those conditions need to be the other way around, if IntlGeneric().IsNull() is true then Intl() will return null.

>+  double value;
>+  if (IntlGeneric().IsAccessible()) {
>+    value = Intl()->MaxValue();
>+  }
>+  else if (IntlGeneric().IsProxy()) {

else if goes on same line as }

also given you have eliminated the other cases the if must be true so else should be fine I believe.

> NS_IMETHODIMP
> xpcAccessibleValue::GetMinimumValue(double* aValue)

Similar comments as the previous function.

> NS_IMETHODIMP
> xpcAccessibleValue::SetCurrentValue(double aValue)

same here

Comment 3

[Michael Li]

Attachment: Make xpcAccessibleValue work with proxied accessibles. r?tbsaunde

Comment 4

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8769356 [details] [diff] [review]
Make xpcAccessibleValue work with proxied accessibles. r?tbsaunde

>-  if (Intl()->IsDefunct())
>+  if (Intl().IsNull())
>     return NS_ERROR_FAILURE;

there should be a blank line here I think

>+  if (Intl().AsAccessible()->IsDefunct())

you don't know Intl().IsAccessible() is true so that isn't safe.

>-  if (Intl()->IsDefunct())
>+  if (Intl().IsNull())
>     return NS_ERROR_FAILURE;
>+  if (Intl().AsAccessible()->IsDefunct())

same here and below.

Comment 5

[Michael Li]

Attachment: Make xpcAccessibleValue work with proxied accessibles. r?tbsaunde

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e87f6e3d61f
Make xpcAccessibleValue work with proxied accessibles. r=tbsaunde

Comment 7

[Michael Li]

Try server results: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8f4fe080b3b9

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9e87f6e3d61f

Comment 9

[Michael Li]

Attachment: Remove unsafe assignment in xpcAccessibleValue::GetMinimumIncrement

Comment 10

[Michael Li]

I missed this error in my first patch, my bad.

Comment 11

[Michael Li]

Attachment: Change MinValue() call to CurValue() in xpcAccessibleValue::GetCurrentValue

Comment 12

[Michael Li]

After these two extra patches are applied, bug 1276721 is fixed and all tests in accessible/tests/browser_caching_values.js pass.

Comment 13

[Marco Zehe (:MarcoZ)]

Now that the dependent bugs have landed, marking this one fixed again. Michael, you should probably mark the obsolete patches obsolete, since you actually dealt with them in the other bugs. :-)