739190 - change AppendTarget(nsIContent* aContent) to take nsDocAccessible* argument

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

change AppendTarget(nsIContent* aContent) (see http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/Relation.h#123) to AppendTarget(nsDocAccessible* aDocument, nsIContent* aContent) and use nsDocAccessible to get an accessible for the content instead GetAccService().

Comment 1

[Ye Kaiqi]

Attachment: patch

Need some comments, thanks!

Comment 2

[Trevor Saunders (:tbsaunde)]

Comment on attachment 611287 [details] [diff] [review]
patch

>     if (aContent)
>-      AppendTarget(GetAccService()->GetAccessible(aContent, nsnull));
>+      AppendTarget(aDocument->GetAccessible(aContent, nsnull));

nsDocAccessible::GetAccessible() only takes one argument.

you might also want to update the callers.

Comment 3

[Oussama BADR]

Hi everyone!

Can I continue what Kaigi began ? 

If it's permitted, so I will recapitulate, the bug is about fixing  AppendTarget as Surkov detailed in the first comment,
and after Kaigi's patch and Trevor's comment the problem was fixed a little bit but we should update nsDocAccessible::GetAccessible(), right?

thanks!

Comment 4

[Mark Capella [:capella]]

Attachment: Patch (v1)

Requesting feedback from mentor ... 

This patch builds and mochitests successfully locally.

Comment 5

[Eitan Isaacson [:eeejay]]

Comment on attachment 621364 [details] [diff] [review]
Patch (v1)

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

Looks straightforward. Thanks!

Comment 6

[Mark Capella [:capella]]

Comment on attachment 621364 [details] [diff] [review]
Patch (v1)

Thank you !

Comment 7

[alexander :surkov (:asurkov)]

Comment on attachment 621364 [details] [diff] [review]
Patch (v1)

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

::: accessible/src/base/Relation.h
@@ +119,5 @@
>    /**
>     * Append the one accessible for this content node to the set of related
>     * accessibles.
>     */
> +  inline void AppendTarget(nsDocAccessible* aDocument, nsIContent* aContent)

nit: you don't need 'inline' (i'll fix it before landing)

Comment 8

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f593f893ebd7

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f593f893ebd7