1483077 - Remove getBrowser() function and replace usages with gBrowser

Status: RESOLVED FIXED

(Firefox :: General, task, P5)

Description

[Tim Nguyen :ntim]

Definition: https://searchfox.org/mozilla-central/rev/2466b82b729765fb0a3ab62f812c1a96a7362478/browser/base/content/browser.js#7344

Uses: https://searchfox.org/mozilla-central/search?q=symbol:%23getBrowser&redirect=false

Comment 1

[Akash Srivastava]

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

Comment 2

[Tim Nguyen :ntim]

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

Comment 3

[Rob Wu [:robwu]]

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).

Comment 4

[Swapnil Rustagi]

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

Comment 5

[Rob Wu [:robwu]]

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

Comment 6

[Swapnil Rustagi]

Thanks a lot for the explanation, working on it.

Comment 7

[Arpit Bharti]

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

Comment 8

[Swapnil Rustagi]

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.

Comment 9

[Arpit Bharti]

Attachment: Bug 1483077 - Replaced reference to getBrowser with gBrowser

Comment 10

[Swapnil Rustagi]

Have you already solved it?

Comment 11

[Rob Wu [:robwu]]

@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.

Comment 12

[Swapnil Rustagi]

@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.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c263feabee1d
Replaced reference to getBrowser with gBrowser r=robwu,dao

Comment 14

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/c263feabee1d