Closed Bug 1159828 Opened 9 years ago Closed 9 years ago

make ia2Hyperlink use proxies

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: tbsaunde, Assigned: tbsaunde)

Details

Attachments

(1 file)

      No description provided.
Attachment #8599419 - Flags: review?(dbolter)
Comment on attachment 8599419 [details] [diff] [review]
make ia2Hyperlink use proxies

Review of attachment 8599419 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/windows/ia2/ia2AccessibleHyperlink.cpp
@@ +97,5 @@
>  
>    VariantInit(aAnchorTarget);
>  
>    Accessible* thisObj = static_cast<AccessibleWrap*>(this);
> +    nsAutoCString uriStr;

nit: too much indent.

@@ +103,5 @@
> +    bool ok;
> +    thisObj->Proxy()->AnchorURIAt(aIndex, uriStr, &ok);
> +    if (!ok)
> +      return S_FALSE;
> +  } else {

nit: please brace the single if block or add a spacer line.

@@ -103,5 @@
>  
> -  nsAutoCString prePath;
> -  nsresult rv = uri->GetPrePath(prePath);
> -  if (NS_FAILED(rv))
> -    return GetHRESULT(rv);

It isn't clear to me why this existed and what removing it will do.

@@ +120,5 @@
>  
> +    nsresult rv = uri->GetSpec(uriStr);
> +    if (NS_FAILED(rv))
> +      return GetHRESULT(rv);
> +  }

ditto

@@ -106,5 @@
> -  if (NS_FAILED(rv))
> -    return GetHRESULT(rv);
> -
> -  nsAutoCString path;
> -  rv = uri->GetPath(path);

And what will removing this do?
> @@ -103,5 @@
> >  
> > -  nsAutoCString prePath;
> > -  nsresult rv = uri->GetPrePath(prePath);
> > -  if (NS_FAILED(rv))
> > -    return GetHRESULT(rv);
> 
> It isn't clear to me why this existed and what removing it will do.
> 
> @@ +120,5 @@
> >  
> > +    nsresult rv = uri->GetSpec(uriStr);
> > +    if (NS_FAILED(rv))
> > +      return GetHRESULT(rv);
> > +  }
> 
> ditto

the full uri is uri.prePath + uri.path so its just a overly compicated way to get the url as a string that is replaced with a simpler one.
Comment on attachment 8599419 [details] [diff] [review]
make ia2Hyperlink use proxies

Review of attachment 8599419 [details] [diff] [review]:
-----------------------------------------------------------------

OK thanks, please fix nits.
Attachment #8599419 - Flags: review?(dbolter) → review+
https://hg.mozilla.org/mozilla-central/rev/603d4c01ca0a
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Assignee: nobody → tbsaunde+mozbugs
You need to log in before you can comment on or make changes to this bug.