Open Bug 1137691 Opened 5 years ago Updated 11 months ago

SpecialPowers.getFocusedElementForWindow is misnamed

Categories

(Testing :: Mochitest, defect)

defect
Not set

Tracking

(Not tracked)

People

(Reporter: enndeakin, Unassigned, Mentored)

Details

(Whiteboard: [good first bug])

Attachments

(1 file)

This function seems to be named after the focus manager getFocusedElementForWindow which it is just a wrapper for, but it returns the focused child window not the focused element. Also, the deep argument should be removed and true implied, since passing false will not just return the window that was passed in.

Or the function should be adjusted to just be a wrapper around getFocusedElementForWindow and return the element.

There's only one caller that I can see though, which only needs the child window, so we could just change the api name and arguments here.
Alternately, if there's just one caller, remove the API entirely and make the caller do it using SpecialPowers.wrap or using SpecialPowers.loadChromeScript if it needs to be e10s-friendly.
Mentor: enndeakin
Whiteboard: [good first bug]
I just got my environment set up using this page https://developer.mozilla.org/en-US/docs/Simple_Firefox_build and if there's still a willing mentor for this bug I would like to work on it. If so I just need to know where to look to get started :) cheers
Yes, one of two options here:

1. Rename the function and any callers. (easier)
2. Remove the function and have the caller use SpecialPowers.wrap (Ted could probably help more here)
As this is my first bug I would prefer the easier route, I have Sublime Text set up to browse through the source code and if you could point me to the right page to start that would be helpful, I'm starting from scratch haha
Tanner, are you still working on this? If not, I would like to pick this up. I have a development VM set up as well.

Thank you very much.
Neil, could I take over this bug? I would like to complete it as part of a software engineering course at the University of Toronto - CSC302.
Hey Zach sorry for the late reply, I am not currently working on this particular bug feel free :)
Booyah! Thanks for following up, Tanner. 

Outside of removing the functionality, are there any tests that should be implemented?
Ted, is it best to just remove the related call? Or refactor it?
Removing the function would probably be better.
If no one is working on this I am wanting to start working on it.
Attached patch rename.patchSplinter Review
Here is a patch for renaming the function and where it is called.
You need to log in before you can comment on or make changes to this bug.