Closed Bug 1483077 Opened Last year Closed 5 months ago

Remove getBrowser() function and replace usages with gBrowser

Categories

(Firefox :: General, task, P5)

task

Tracking

()

RESOLVED FIXED
Firefox 68
Tracking Status
firefox68 --- fixed

People

(Reporter: ntim, Assigned: arpit73, Mentored)

References

Details

(Keywords: good-first-bug)

Attachments

(1 file)

Priority: -- → P5
Keywords: good-first-bug

Hello, I am Akash, I am new to this community. Can I get assigned to this bug?

Sorry, I don’t think this is a good first bug. You can try bug 1516536 or bug 1519920 instead.

Keywords: good-first-bug

I believe that this is a good-first-bug. What else do you think that needs to be done besides replacing getBrowser() with gBrowser and removing the definition of getBrowser() ?

The only potential complication is the definition of an unrelated getBrowser method at https://searchfox.org/mozilla-central/rev/14dc5b7d8a6da1854b2f9f33f1da77a97368cd54/docshell/test/chrome/docshell_helpers.js#478-482,486-488 . This file can simply be ignored.

So to fix this bug:

  1. Find the files that one needs to modify with https://searchfox.org/mozilla-central/search?q=%5Cbgetbrowser%5C(%5C)&case=false&regexp=true&path=js (ignor docshell/test/chrome/docshell_helpers.js)
  2. Replace getBrowser() with gBrowser, and remove the definition of getBrowser.
  3. Run the modified tests to see if the basics work as intended.
  4. Ask the mentor to run all tests on the try server to see if there are any other unexpected changes (for example because new code was checked in that used getBrowser).
Mentor: rob
Keywords: good-first-bug

Hi! Since this is labelled as a good-first-bug, I would like to work on this to get familiar with how to contribute.

Flags: needinfo?(rob)

Hi Swapnil,

To get started you need to set up a development environment, that consists of a checkout of the Firefox source code and the tools to build it. You can find instructions to do so at https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build

Once you have checked out the source code, follow the steps from comment 3 to write the patch.
Step 3, "run the modified tests" can be done (after building via mach build) by running mach test [one or more paths to test file]. For example, to run the first test from the list of tests that you have to modify, run:

mach test browser/base/content/test/general/browser_scope.js

If you are ready to have your patches reviewed, submit a patch by folloing the instructions at https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch

Flags: needinfo?(rob)

Thanks a lot for the explanation, working on it.

Since there hasn't been any update in a while, can this be assigned to me?

Hi Arpit, I had some issues with the development environment, so could not submit a patch. However, I realize I am holding this up, so if I am unable to submit a patch in a day, please claim it.

Have you already solved it?

@Arpit Thanks for your patch! In the future, please allow for a day or two for someone else to respond before claiming a bug that had been claimed before.

@Swapnil
Arpit has already submitted a patch that went through some review. So I'll assign the bug to him, even though you were earlier.
If you want to find other good first bugs, take a look at https://codetribute.mozilla.org/
And don't hesitate asking questions if you ever get stuck (by mail, bugzilla, IRC, Slack, etc.), whether it's about the direction of the patch, or setting up the development environment.

Assignee: nobody → arpitbharti73

@Rob Wu
Yes, surely I will communicate if I am facing any issues in future. Thank you for your time and comments regarding testing and submitting a patch.

Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c263feabee1d
Replaced reference to getBrowser with gBrowser r=robwu,dao
Type: enhancement → task
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
Regressions: 1546075
You need to log in before you can comment on or make changes to this bug.