1286322 - Make xpcAccessibleHyperLink work with proxied accessibles.

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Comment 1

[Michael Li]

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

Comment 2

[Trevor Saunders (:tbsaunde)]

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

> xpcAccessibleHyperLink::GetURI(int32_t aIndex, nsIURI** aURI)
> {
>   NS_ENSURE_ARG_POINTER(aURI);
> 
>-  if (!Intl())
>+  if (Intl().IsNull())
>     return NS_ERROR_FAILURE;
> 
>-  if (aIndex < 0 || aIndex >= static_cast<int32_t>(Intl()->AnchorCount()))
>+  if (aIndex < 0)
>     return NS_ERROR_INVALID_ARG;
> 
>-  RefPtr<nsIURI>(Intl()->AnchorURIAt(aIndex)).forget(aURI);
>+  if (Intl().IsAccessible()) {
>+    if (aIndex >= static_cast<int32_t>(Intl().AsAccessible()->AnchorCount()))
>+      return NS_ERROR_INVALID_ARG;
>+
>+    RefPtr<nsIURI>(Intl().AsAccessible()->AnchorURIAt(aIndex)).forget(aURI);
>+  } else {
>+    bool isCountValid = false;
>+    int32_t anchorCount =
>+      static_cast<int32_t>(Intl().AsProxy()->AnchorCount(&isCountValid));
>+    if (!isCountValid)
>+      return NS_ERROR_FAILURE;

I'd rather not make two ipc calls in the same function.  I don't think you really need to check the number of links first, and you can just try to get the ith one.

>+    nsCString URIStr;

naming it spec might be more conventional.

>+    bool isURIValid = false;
>+    Intl().AsProxy()->AnchorURIAt(aIndex, URIStr, &isURIValid);
>+    if (!isURIValid)
>+      return NS_ERROR_FAILURE;
>+
>+    nsCOMPtr<nsIURI> newURI;

i'd name it just uri.

>+    nsresult newURIResult = NS_NewURI(getter_AddRefs(newURI), URIStr);

just rv.

>+    if (NS_FAILED(newURIResult))
>+      return NS_ERROR_FAILURE;
>+
>+    newURI.forget(aURI);
>+

drop the blank line

> xpcAccessibleHyperLink::GetAnchor(int32_t aIndex, nsIAccessible** aAccessible)
> {
>   NS_ENSURE_ARG_POINTER(aAccessible);
>   *aAccessible = nullptr;
> 
>-  if (!Intl())
>+  if (Intl().IsNull())
>     return NS_ERROR_FAILURE;
>-
>-  if (aIndex < 0 || aIndex >= static_cast<int32_t>(Intl()->AnchorCount()))
>+  
>+  if (aIndex < 0)
>     return NS_ERROR_INVALID_ARG;
> 
>-  NS_IF_ADDREF(*aAccessible = ToXPC(Intl()->AnchorAt(aIndex)));
>+  if (Intl().IsAccessible()) {
>+    if (aIndex >= static_cast<int32_t>(Intl().AsAccessible()->AnchorCount()))
>+      return NS_ERROR_INVALID_ARG;
>+
>+    NS_IF_ADDREF(*aAccessible = ToXPC(Intl().AsAccessible()->AnchorAt(aIndex)));
>+  } else {
>+    bool isCountValid = false;
>+    int32_t anchorCount =
>+      static_cast<int32_t>(Intl().AsProxy()->AnchorCount(&isCountValid));
>+    if (!isCountValid)
>+      return NS_ERROR_FAILURE;

I think you can skip checking the count here too.

Comment 3

[Michael Li]

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

Comment 4

[Michael Li]

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

This version keeps the extra index checks for GetURI() and GetAnchor() if it's an Accessible.

Comment 5

[Trevor Saunders (:tbsaunde)]

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

> xpcAccessibleHyperLink::GetURI(int32_t aIndex, nsIURI** aURI)
> {
>+    nsCOMPtr<nsIURI> uri;
>+    nsresult rv = NS_NewURI(getter_AddRefs(uri), spec);
>+    if (NS_FAILED(rv))
>+      return NS_ERROR_FAILURE;

just use NS_ENSURE_SUCCESS(rv, rv);

Comment 6

[Michael Li]

Attachment: Make xpcAccessibleHyperLink work with proxied accessibles.

Comment 7

[Pulsebot]

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

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3bbf5d4f5e79