Closed Bug 461922 Opened 11 years ago Closed 11 years ago

remove nsPIAccessibleDocument

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: surkov, Assigned: surkov)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #384820 - Flags: superreview?(neil)
Attachment #384820 - Flags: review?(marco.zehe)
Attachment #384820 - Flags: review?(bolterbugz)
A few nits from nsDocAccessible.h:
>+   * Fire accessible event in timeout.

Either "with timeout" or "after timeout".

>+   * accessible children and and remove the accessible object and any decendents

"and and" is doubled, and should be "descendants".

>+   * Proccess the case when anchor was clicked.

"process"

>+   * Used to flush pending events, called in timeout. See FlushPendingEvents.

Also "with timeout" or "after timeout".

>+   * Iterates through sub documents and shutdowns them.

"shuts them down".
thanks, Marco. I fixed this locally.
Attachment #384820 - Attachment is obsolete: true
Attachment #384820 - Flags: superreview?(neil)
Attachment #384820 - Flags: review?(marco.zehe)
Attachment #384820 - Flags: review?(bolterbugz)
Comment on attachment 384820 [details] [diff] [review]
patch

cancelling review since some things become broken.
Attached patch patch2Splinter Review
Attachment #384828 - Flags: review?(marco.zehe)
Attachment #384828 - Flags: review?(bolterbugz)
Attachment #384828 - Flags: review?(marco.zehe) → review+
Comment on attachment 384828 [details] [diff] [review]
patch2

r=me for the functional part. This one doesn't break NVDA like the first one did.
Attachment #384828 - Flags: superreview?(neil)
Attachment #384828 - Flags: superreview?(neil) → superreview+
Comment on attachment 384828 [details] [diff] [review]
patch2

>+  nsRefPtr<nsDocAccessible> docAcc =
>+    nsAccUtils::QueryAccessibleDocument(accessibleDoc);
>+  if (!docAcc)
>     return NS_OK;
>-  }
>-  return privateAccessibleDoc->FireAnchorJumpEvent();
>+
>+  docAcc->FireAnchorJumpEvent();
>+  return NS_OK;
More easily written as:
nsRefPtr<nsDocAccessible> docAcc =
  nsAccUtils::QueryAccessibleDocument(accessibleDoc);
if (docAcc)
  docAcc->FireAnchorJumpEvent();
return NS_OK;
(Similarly for some of the following methods.)

>+#define NS_INTERFACE_MAP_STATIC_AMBIGUOUS(_class) \
>+  if (aIID.Equals(NS_GET_IID(_class))) { \
>+    NS_ADDREF(this); \
>+    *aInstancePtr = this; \
>+    return NS_OK; \
>+  } else
There seems to be a number of approaches to this!
It might be worth filing an XPCOM bug on creating a consistent version.
Comment on attachment 384828 [details] [diff] [review]
patch2

r=me, I have no additional comments.
Attachment #384828 - Flags: review?(bolterbugz) → review+
checked in with Neil's comment addressed on 1.9.2 - http://hg.mozilla.org/mozilla-central/rev/662379876512
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
(In reply to comment #7)

> There seems to be a number of approaches to this!
> It might be worth filing an XPCOM bug on creating a consistent version.

I filed bug 500347.
You need to log in before you can comment on or make changes to this bug.