Closed Bug 1207862 Opened 9 years ago Closed 9 years ago

allow retrieving proxied accessibles by id

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: tbsaunde, Assigned: tbsaunde)

References

Details

Attachments

(5 files)

      No description provided.
While we're reorganizing this function move the simple cases to the beginning,
      and make lookup in the document tree last.
Attachment #8665193 - Flags: review?(dbolter)
Soon we will need to iterate over all of the top level remote documents.
Attachment #8665194 - Flags: review?(dbolter)
Assignee: nobody → tbsaunde+mozbugs
Blocks: e10sa11y2
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.
Attachment #8665193 - Flags: review?(dbolter) → review+
Attachment #8665194 - Flags: review?(dbolter) → review+
Attachment #8665196 - Flags: review?(dbolter) → review+
Attachment #8665197 - Flags: review?(dbolter) → review+
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).
Attachment #8665198 - Flags: review?(dbolter) → review+
Possible this change caused bug 1208680 ?
No longer depends on: 1208680
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: