Closed Bug 1215657 Opened 4 years ago Closed 4 years ago

make a bunch more of msaa work with proxied accessibles

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: tbsaunde, Assigned: tbsaunde)

Details

Attachments

(6 files)

No description provided.
Assignee: nobody → tbsaunde+mozbugs
Attachment #8675089 - Flags: review?(dbolter) → review+
Attachment #8675090 - Flags: review?(dbolter) → review+
Attachment #8675091 - Flags: review?(dbolter) → review+
Attachment #8675092 - Flags: review?(dbolter) → review+
Comment on attachment 8675093 [details] [diff] [review]
make AccessibleWrap::accDoDefaultAction work with proxies

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

::: accessible/windows/msaa/AccessibleWrap.cpp
@@ +1138,1 @@
>  

I think you missed a 'Proxy()->' in that chain?
Comment on attachment 8675095 [details] [diff] [review]
make AccessibleWrap::get_accSelection work with proxies

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

::: accessible/windows/msaa/AccessibleWrap.cpp
@@ +839,5 @@
> +
> +      uint32_t selectedCount = proxies.Length();
> +      for (uint32_t i = 0; i < selectedCount; i++) {
> +        selectedItems.AppendElement(WrapperFor(proxies[i]));
> +      }

Can you add a comment for why we need to do the loop part?
(In reply to David Bolter [:davidb] from comment #7)
> Comment on attachment 8675093 [details] [diff] [review]
> make AccessibleWrap::accDoDefaultAction work with proxies
> 
> Review of attachment 8675093 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/windows/msaa/AccessibleWrap.cpp
> @@ +1138,1 @@
> >  
> 
> I think you missed a 'Proxy()->' in that chain?

yes, oops


(In reply to David Bolter [:davidb] from comment #8)
> Comment on attachment 8675095 [details] [diff] [review]
> make AccessibleWrap::get_accSelection work with proxies
> 
> Review of attachment 8675095 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/windows/msaa/AccessibleWrap.cpp
> @@ +839,5 @@
> > +
> > +      uint32_t selectedCount = proxies.Length();
> > +      for (uint32_t i = 0; i < selectedCount; i++) {
> > +        selectedItems.AppendElement(WrapperFor(proxies[i]));
> > +      }
> 
> Can you add a comment for why we need to do the loop part?

it seems rather obvious its just converting an array of x to an array of y, because we need an array of y later?  There's really nothing at all interesting here that justifies comments.
Comment on attachment 8675093 [details] [diff] [review]
make AccessibleWrap::accDoDefaultAction work with proxies

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

OK r+ assuming that fix.
Attachment #8675093 - Flags: review?(dbolter) → review+
Attachment #8675095 - Flags: review?(dbolter) → review+
You need to log in before you can comment on or make changes to this bug.