Trevor Saunders (:tbsaunde) Assignee
	Description • 9 years ago

      No description provided.

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 7 • 9 years ago

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?

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 8 • 9 years ago

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?

	Trevor Saunders (:tbsaunde) Assignee
	Comment 9 • 9 years ago

(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.

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 10 • 9 years ago

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.

	Pulsebot
	Comment 11 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/afd950962226
https://hg.mozilla.org/integration/mozilla-inbound/rev/7abd3e884ece
https://hg.mozilla.org/integration/mozilla-inbound/rev/a95b283ec1e4
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3e04e9203dd
https://hg.mozilla.org/integration/mozilla-inbound/rev/1db223020327
https://hg.mozilla.org/integration/mozilla-inbound/rev/d735fd6df22a

	Carsten Book [:Tomcat]
	Comment 12 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/afd950962226
https://hg.mozilla.org/mozilla-central/rev/7abd3e884ece
https://hg.mozilla.org/mozilla-central/rev/a95b283ec1e4
https://hg.mozilla.org/mozilla-central/rev/e3e04e9203dd
https://hg.mozilla.org/mozilla-central/rev/1db223020327
https://hg.mozilla.org/mozilla-central/rev/d735fd6df22a