Obsolete unique id generated by sdnAccessible::get_nodeInfo

RESOLVED FIXED in Firefox 55

Status

()

Core
Disability Access APIs
RESOLVED FIXED
11 months ago
11 months ago

People

(Reporter: aklotz, Assigned: aklotz)

Tracking

Trunk
mozilla55
Unspecified
Windows
Points:
---

Firefox Tracking Flags

(firefox53 wontfix, firefox54 wontfix, firefox55 fixed)

Details

(Whiteboard: [aes+][JAWS])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

11 months ago
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.
(Assignee)

Comment 1

11 months ago
While this is not strictly an e10s bug, it impacts e10s JAWS testing enough such that it should block.
Whiteboard: [aes+][JAWS]
(Assignee)

Updated

11 months ago
status-firefox53: --- → affected
status-firefox54: --- → affected
status-firefox55: --- → affected
(Assignee)

Updated

11 months ago
Summary: Obsolte unique id generated by sdnAccessible::get_nodeInfo → Obsolete unique id generated by sdnAccessible::get_nodeInfo
(Assignee)

Comment 2

11 months ago
Created attachment 8867933 [details] [diff] [review]
Make sdnAccessible use the same child ID scheme as AccessibleWrap when it does not have an associated accessible
Assignee: nobody → aklotz
Status: NEW → ASSIGNED
Attachment #8867933 - Flags: review?(tbsaunde+mozbugs)
(Assignee)

Comment 3

11 months ago
Created attachment 8867934 [details] [diff] [review]
Make sdnAccessible use the same child ID scheme as AccessibleWrap when it does not have an associated accessible (r2)
Attachment #8867933 - Attachment is obsolete: true
Attachment #8867933 - Flags: review?(tbsaunde+mozbugs)
Attachment #8867934 - Flags: review?(tbsaunde+mozbugs)
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_
Attachment #8867934 - Flags: review?(tbsaunde+mozbugs) → review+
(Assignee)

Comment 5

11 months ago
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
https://hg.mozilla.org/mozilla-central/rev/9717295b3da5
Status: ASSIGNED → RESOLVED
Last Resolved: 11 months ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
status-firefox53: affected → wontfix

Comment 7

11 months ago
Mark 54 won't fix as this is late for Beta54.
status-firefox54: affected → wontfix
You need to log in before you can comment on or make changes to this bug.