1207862 - allow retrieving proxied accessibles by id

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Comment 1

[Trevor Saunders (:tbsaunde)]

Attachment: refactor GetXPAccessibleFor() so proxies and non proxies are handle in the same place for each type of id

While we're reorganizing this function move the simple cases to the beginning,
      and make lookup in the document tree last.

Comment 2

[Trevor Saunders (:tbsaunde)]

Attachment: Add DocManager::TopLevelRemoteDocs()

Soon we will need to iterate over all of the top level remote documents.

Comment 3

[Trevor Saunders (:tbsaunde)]

Attachment: make it possible to traverse down the tree of proxied documents

Comment 4

[Trevor Saunders (:tbsaunde)]

Attachment: make WrapperFor() take a const ProxyAccessible *

Comment 5

[Trevor Saunders (:tbsaunde)]

Attachment: make AccessibleWrap::GetXPAccessibleFor() return the AccessibleWrap for proxies

Comment 6

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 8665193 [details] [diff] [review]
refactor GetXPAccessibleFor() so proxies and non proxies are handle in the same place for each type of id

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

::: accessible/windows/msaa/AccessibleWrap.cpp
@@ +1452,4 @@
>  #ifdef _WIN64
>      GetAccessibleInSubtree(document, static_cast<uint32_t>(aVarChild.lVal));
>  #else
> +  document->GetAccessibleByUniqueIDInSubtree(uniqueID);

nit: needs 2 more spaces.

Comment 7

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 8665198 [details] [diff] [review]
make AccessibleWrap::GetXPAccessibleFor() return the AccessibleWrap for proxies

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

OK. Please see/fix comments before landing.

::: accessible/windows/msaa/AccessibleWrap.cpp
@@ +1462,5 @@
>    // Convert child ID to unique ID.
> +  // First handle the case that both this accessible and the id'd one are in
> +  // this process.
> +  if (!IsProxy()) {
> +      void* uniqueID = reinterpret_cast<void*>(-aVarChild.lVal);

nit: This whole if block has 2 too many spaces and other formatting issues.

@@ +1481,5 @@
> +      // ARIA documents and popups).
> +      Accessible* parent = child;
> +      while (parent && parent != document) {
> +      if (parent == this)
> +      return child;

nit: the while, if, and if block need relative indenting.

@@ +1509,5 @@
> +  }
> +
> +  // Finally we need to handle the case that this accessible is in the main
> +  // process, but the target is proxied.  This is the case when the target
> +  // accessible is in a different document from this one.

We chatted and you mentioned changing this last comment to 'child document'.

@@ +1513,5 @@
> +  // accessible is in a different document from this one.
> +  DocAccessibleParent* proxyDoc = nullptr;
> +  DocAccessible* doc = Document();
> +  const nsTArray<DocAccessibleParent*>* remoteDocs =
> +    DocManager::TopLevelRemoteDocs();

We chatted and think we need an early bailout for remoteDocs null.

@@ +1529,5 @@
> +    Accessible* parent = outerDoc;
> +    while (parent && parent != doc) {
> +      if (parent == this) {
> +        AccessibleWrap* proxyWrapper =
> +          GetProxiedAccessibleInSubtree(remoteDocs->ElementAt(i), id);

nit: could use a variable for remoteDocs->ElementAt(i) (up at top of for loop).

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2849177a50a1
https://hg.mozilla.org/integration/mozilla-inbound/rev/f2260d4a6b08
https://hg.mozilla.org/integration/mozilla-inbound/rev/222b880291e0
https://hg.mozilla.org/integration/mozilla-inbound/rev/33983344a8d9
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9bb88c728f9

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2849177a50a1
https://hg.mozilla.org/mozilla-central/rev/f2260d4a6b08
https://hg.mozilla.org/mozilla-central/rev/222b880291e0
https://hg.mozilla.org/mozilla-central/rev/33983344a8d9
https://hg.mozilla.org/mozilla-central/rev/e9bb88c728f9

Comment 10

[Jim Jeffery not reading bug-mail 1/2/11]

Possible this change caused bug 1208680 ?