1365073 - Obsolete unique id generated by sdnAccessible::get_nodeInfo

Status: RESOLVED FIXED

(Core :: Disability Access APIs, enhancement)

Description

[(No longer employed by Mozilla) Aaron Klotz]

If there is no accessible associated with the ISimpleDOMNode, we do the old negation method.

Not only is that method no longer used, it is also bad for pointers in the 3GB+ range.

While watching a11y event traces I have seen strange child IDs that should not be possible under the new scheme, so I suspect I was seeing an occurrence of this bug.

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

While this is not strictly an e10s bug, it impacts e10s JAWS testing enough such that it should block.

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Make sdnAccessible use the same child ID scheme as AccessibleWrap when it does not have an associated accessible

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Make sdnAccessible use the same child ID scheme as AccessibleWrap when it does not have an associated accessible (r2)

Comment 4

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8867934 [details] [diff] [review]
Make sdnAccessible use the same child ID scheme as AccessibleWrap when it does not have an associated accessible (r2)

> AccessibleWrap::~AccessibleWrap()
> {
>   if (mID != kNoID) {
>-    sIDGen.ReleaseID(this);
>+    sIDGen.ReleaseID(WrapNotNull(this));

it would have been nice to split this api change to its own patch.

>+/* static */ void
>+AccessibleWrap::AssignChildIDTo(NotNull<sdnAccessible*> aSdnAcc)
>+{
>+  aSdnAcc->SetUniqueID(sIDGen.GetID());
>+}
>+
>+/* static */ void
>+AccessibleWrap::ReleaseChildID(NotNull<sdnAccessible*> aSdnAcc)
>+{
>+  sIDGen.ReleaseID(aSdnAcc);

honestly I'd rather just make sIDGen global that doesn't really seem like a layering violation its just a handy spot to dump it, so seems fine to expose raw access.

>   nsCOMPtr<nsINode> mNode;
>   RefPtr<AccessibleWrap> mWrap;
>+  Maybe<uint32_t> mUniqueId;

that's kind of inefficient since None isn't packed as the int being 0, but its cleaner and I don't really care about memory usage of this.
> };
> 
> } // namespace a11y
> } // namespace mozilla
> 
> #endif // mozilla_a11y_sdnAccessible_h_

Comment 5

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9717295b3da5b3227e19cf90f9b4a6670a7e0292
Bug 1365073: Modify sdnAccessible to always generate unique IDs using AccessibleWrap's scheme even when it is not associated with an accessible; r=tbsaunde

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/9717295b3da5

Comment 7

[Gerry Chang [:gchang]]

Mark 54 won't fix as this is late for Beta54.