Kill off nsDOMWindowUtils::FindElementWithViewId

RESOLVED FIXED in Firefox 47

Status

()

Core
Layout
P5
trivial
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: kats, Assigned: Jeffrey Tran, Mentored)

Tracking

unspecified
mozilla47
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox47 fixed)

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment)

This function is only used in one place, and that can be easily fixed. The reason I want to get rid of this function is that it's a footgun because it special-cases the ROOT_SCROLL_ID and can return a different value from nsLayoutUtils::FindIDFor for it.

Updated

4 years ago
Severity: normal → trivial
Priority: -- → P5
This function isn't used any more, it's should be ok to just delete it from nsIDOMWindowUtils.idl and nsDOMWindowUtils.cpp. It's a simple first bug, I can mentor it. The only 'trick' here is to bump the UUID in the IDL file.
Mentor: bugmail.mozilla@staktrace.com
Whiteboard: [good first bug]
(Assignee)

Comment 2

2 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #1)
> This function isn't used any more, it's should be ok to just delete it from
> nsIDOMWindowUtils.idl and nsDOMWindowUtils.cpp. It's a simple first bug, I
> can mentor it. The only 'trick' here is to bump the UUID in the IDL file.

Hi, can you explain how to "bump the UUID"?
The easiest way is to run |mach uuid| to generate a new UUID and replace the existing one (e.g. at [1]) with the new one.

[1] https://dxr.mozilla.org/mozilla-central/rev/aa90f482e16db77cdb7dea84564ea1cbd8f7f6b3/dom/interfaces/base/nsIDOMWindowUtils.idl#52
.. although I think you might not need to do that anymore, according to the discussion at https://groups.google.com/forum/#!topic/mozilla.dev.platform/n6qBpyxoI6I
(Assignee)

Comment 5

2 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4)
> .. although I think you might not need to do that anymore, according to the
> discussion at
> https://groups.google.com/forum/#!topic/mozilla.dev.platform/n6qBpyxoI6I

So then do we just delete the function and not worry about the UUID?
Yup!
(Assignee)

Comment 7

2 years ago
Created attachment 8716417 [details] [diff] [review]
rev1 - Remove FindElementWithViewId from nsIDOMWindowUtils.idl and nsDOMWindowUtils.cpp.
Attachment #8716417 - Flags: review?(bugmail.mozilla)
Assignee: nobody → tranj23
Comment on attachment 8716417 [details] [diff] [review]
rev1 - Remove FindElementWithViewId from nsIDOMWindowUtils.idl and nsDOMWindowUtils.cpp.

Review of attachment 8716417 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks! I'll land this in a bit.
Attachment #8716417 - Flags: review?(bugmail.mozilla) → review+

Comment 10

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/00b5b6c9f887
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Jeffrey, thank you for the patch!
(Assignee)

Comment 12

2 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #11)
> Jeffrey, thank you for the patch!

I'm happy to help!
You need to log in before you can comment on or make changes to this bug.