teach IAccessible impl about proxy wrappers

RESOLVED FIXED in Firefox 38

Status

()

Core
Disability Access APIs
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: tbsaunde, Assigned: tbsaunde)

Tracking

(Blocks: 1 bug)

unspecified
mozilla38
x86_64
Linux
Points:
---

Firefox Tracking Flags

(firefox38 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Comment 1

3 years ago
Created attachment 8556574 [details] [diff] [review]
teach IAccessible impl about proxy wrappers
Attachment #8556574 - Flags: review?(dbolter)
Comment on attachment 8556574 [details] [diff] [review]
teach IAccessible impl about proxy wrappers

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

r=me with inlines. Also I'd be more comfortable having TODO comments for the E_NOTIMPL blocks; with bonus marks if they refer to filed bug numbers. BTW I didn't know (or forgot) we had to worry about negative child IDs (?)

::: accessible/ipc/ProxyAccessible.cpp
@@ +51,5 @@
>  }
>  
> +bool
> +ProxyAccessible::MustPruneChildren() const
> +{

Cut and paste code is always hard to swallow but I'm not sure what else you could do here. Maybe adding a comment that this should be kept in sync with sister function would be good.

::: accessible/windows/msaa/AccessibleWrap.cpp
@@ +405,5 @@
>  
> +  a11y::role geckoRole;
> +  if (IsProxy()) {
> +    geckoRole = Proxy()->Role();
> +} else {

The above line needs to be indented.

@@ +413,4 @@
>  #endif
>  
> +    geckoRole = xpAccessible->Role();
> +}

The } needs to be indented.

@@ +453,5 @@
>      return S_OK;
>    }
>  
> +if (IsProxy())
> +  return E_FAIL;

Please indent the above two lines.
Attachment #8556574 - Flags: review?(dbolter) → review+
(Assignee)

Comment 3

3 years ago
Created attachment 8561598 [details] [diff] [review]
teach IAccessible impl about proxy wrappers

fixed up a couple things
Attachment #8556574 - Attachment is obsolete: true
Attachment #8561598 - Flags: review?(dbolter)
Comment on attachment 8561598 [details] [diff] [review]
teach IAccessible impl about proxy wrappers

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

thanks.

::: accessible/ipc/ProxyAccessible.cpp
@@ +52,5 @@
>  
> +bool
> +ProxyAccessible::MustPruneChildren() const
> +{
> +  // this is the equivelent to nsAccUtils::MustPrune for proxies and should be

nit: "equivelent" should be "equivalent"
Attachment #8561598 - Flags: review?(dbolter) → review+
(Assignee)

Comment 5

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/baf1a5507fd6
https://hg.mozilla.org/mozilla-central/rev/baf1a5507fd6
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Assignee: nobody → tbsaunde+mozbugs
You need to log in before you can comment on or make changes to this bug.