Remove Accessible::DOMNode

RESOLVED FIXED in Firefox 61

Status

()

P5
enhancement
RESOLVED FIXED
6 years ago
a year ago

People

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

Tracking

(Blocks: 1 bug, {good-first-bug})

Trunk
mozilla61
good-first-bug
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox61 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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

Comment 7

a year ago
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
(Assignee)

Comment 9

a year ago
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
(Assignee)

Comment 12

a year ago
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
(Assignee)

Comment 14

a year ago
Posted patch Remove Accessible::DOMNode (obsolete) — Splinter Review
replace Accessible::DOMNode with Accessible::GetNode version

MozReview-Commit-ID: 9p74ISHnWzF
(Assignee)

Comment 15

a year ago
I'm sorry but some extra file was included in the patch. I'll try it again.
(Assignee)

Comment 16

a year ago
Posted patch Remove Accessible::DOMNode (obsolete) — Splinter Review
replace Accessible::DOMNode with Accessible::GetNode version
(Assignee)

Comment 17

a year ago
Oh, I'm so sorry again, an extra character 'S' was included.
(Assignee)

Comment 18

a year ago
replace Accessible::DOMNode with Accessible::GetNode version
Attachment #8961304 - Attachment is obsolete: true
Attachment #8961301 - Attachment is obsolete: true
Keywords: checkin-needed

Comment 19

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a5a9d5bca98b
Remove Accessible::DOMNode. r=surkov
Keywords: checkin-needed

Comment 20

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a5a9d5bca98b
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox61: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.