Closed Bug 865615 Opened 7 years ago Closed 2 years ago

Remove Accessible::DOMNode

Categories

(Core :: Disability Access APIs, enhancement, P5)

enhancement

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: ayg, Assigned: dora.tokio, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: good-first-bug)

Attachments

(1 file, 2 obsolete files)

iirc we wanted to replace it on raw pointer version (I guess via nsINode::AsDOMNode())
I think there's only two or so uses, so it's probably easier to deal in the callers.
Will we still have the ability to reference the original DOM node from the accessible? I believe some of the JSAT folder uses, or new patches plan to use, this to gain access to the original node coming from the accessible object.
GetNode().
jsat is xpcom based, this bug is about internal method
(In reply to :Ms2ger from comment #2)
> I think there's only two or so uses, so it's probably easier to deal in the
> callers.

I don't mind
Mentor: surkov.alexander
Keywords: good-first-bug
Priority: -- → P5
Can anyone please explain this bug. I would like to work on it.
(In reply to Videet Singhai from comment #7)
> Can anyone please explain this bug. I would like to work on it.

technically, you should replace Accessible:DOMNode() instances on more appropriate calls like GetContent or GetNode().

https://searchfox.org/mozilla-central/source/accessible/generic/Accessible.h#169
Is nobody assigned to this bug yet?
(In reply to dora.tokio from comment #9)
> Is nobody assigned to this bug yet?

yup,nobady mean that no body is assigned
yep, feel free to claim it if you are keen to work on it
Thanks, I'd like to work on it.

Accessible::DOMNode() is called only twice, in a similar situation. So, I rewrite XULTabAccessible.cpp as follows, is this correct?

Before:
>  nsCOMPtr<nsIDOMNode> domNode(DOMNode());
>  nsCOMPtr<nsIDOMNode> tabpanelNode;
>  tabsElm->GetRelatedElement(domNode, getter_AddRefs(tabpanelNode));

After:
>  nsCOMPtr<nsIDOMNode> domNode(do_QueryInterface(GetNode()));
>  nsCOMPtr<nsIDOMNode> tabpanelNode;
>  tabsElm->GetRelatedElement(domNode, getter_AddRefs(tabpanelNode));
yep, sounds good
Assignee: nobody → dora.tokio
Attached patch Remove Accessible::DOMNode (obsolete) — Splinter Review
replace Accessible::DOMNode with Accessible::GetNode version

MozReview-Commit-ID: 9p74ISHnWzF
I'm sorry but some extra file was included in the patch. I'll try it again.
Attached patch Remove Accessible::DOMNode (obsolete) — Splinter Review
replace Accessible::DOMNode with Accessible::GetNode version
Oh, I'm so sorry again, an extra character 'S' was included.
replace Accessible::DOMNode with Accessible::GetNode version
Attachment #8961304 - Attachment is obsolete: true
Attachment #8961301 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/a5a9d5bca98b
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.