Make xpcAccessibleHyperLink work with proxied accessibles.

RESOLVED FIXED in Firefox 50

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: michael.li11702, Assigned: michael.li11702)

Tracking

unspecified
mozilla50
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

No description provided.
Attachment #8770236 - Flags: review?(tbsaunde+mozbugs)
Assignee: nobody → mili
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.
Attachment #8770236 - Flags: review?(tbsaunde+mozbugs)
Attachment #8770619 - Flags: review?(tbsaunde+mozbugs)
Attachment #8770236 - Attachment is obsolete: true
This version keeps the extra index checks for GetURI() and GetAnchor() if it's an Accessible.
Attachment #8770680 - Flags: review?(tbsaunde+mozbugs)
Attachment #8770619 - Attachment is obsolete: true
Attachment #8770619 - Flags: review?(tbsaunde+mozbugs)
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);
Attachment #8770680 - Flags: review?(tbsaunde+mozbugs) → review+
Attachment #8770680 - Attachment is obsolete: true
Keywords: checkin-needed
Summary: Make xpcAccessibleHyperLink work with proxied accessibles. r?tbsaunde → Make xpcAccessibleHyperLink work with proxied accessibles.
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3bbf5d4f5e79
Make xpcAccessibleHyperLink work with proxied accessibles. r=tbsaunde
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/3bbf5d4f5e79
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.