Contribution to Mozilla Firefox
It looks like my every second post on Medium is about a contribution to Mozilla’s projects. I have already worked on Firefox add-ons website and Firefox Focus, and you can read about that in the following posts:
This story is about a contribution to Mozilla add-ons website.medium.com
I continue my journey in the world of Open Source Software, and I would like to share my recent experience of fixing a…medium.com
This story is about my contribution to an upcoming release of Firefox Focus for iOS.medium.com
In this post, I am going to tell you about my experience of working on Mozilla’s flagship product which is Firefox 🔥🦊
The problem was that the codebase of Firefox is so large (it is the biggest codebase I’ve ever worked with) that Mozilla couldn’t roll out ESLint across the whole code. Thus, they put some directories with code into the .eslintignore file, so the linter does not complain while building the code. You can see the whole list of directories and files that haven’t adopted ESLint yet in the following file:
Always ignore node_modules. **/node_modules/**/*.* # Always ignore crashtests - specially crafted files that originally…searchfox.org
As of today, there are about 350 files/directories that aren’t covered by ESLint.
I took an issue that asked to enable ESLint for dom/crypto folder.
First of all, I had to set up a project on my local machine. It was a new experience for me because Mozilla uses Mercurial as their primary source control tool (you can still use Git, but it’s more tricky). However, I found that Mercurial workflow and commands are very much like Git.
I’m using macOS, and here are the instructions to set up Firefox on your macOS device:
- Install Mercurial:
brew install mercurial
2. Get Firefox source code using Mercurial:
hg clone https://hg.mozilla.org/mozilla-central
It took me about an hour to download it!
3. Install all dependencies and build the project:
Build command will take about an hour to complete.
NOTE: you might not need to build the project; it depends on your issue.
After the local environment is set up, I could start hacking on the issue. I removed dom/crypto folder from .eslintignore file and then ran the following command to see eslint errors:
./mach eslint dom/crypto
Here is the output I got:
ESLint caught about 500 errors!
Fortunately, I was able to fix some of the errors by ESLint’s autofix:
./mach eslint --fix dom/crypto
This command fixed some of the errors and following the instructions I was given, I had to make a commit for automatic fixes:
hg commit -m "Bug 1508825 Enable ESLint for dom/crypto (automatic changes)"
git commit command.
TIP: Make sure you don’t have # in front of the bug number (i.e. commit message like “Bug #1508825 Enable ESLint…” wouldn’t work). It’s important because otherwise when you try to submit your fixes, moz-phab (Mozilla’s submission tool) will not recognize the bug number. In this case, you would have to edit your commit history and change the commit message.
That’s one of the mistakes I’ve done by myself, and I hope this tip would help somebody to avoid it in the future.
Next, I started fixing linting errors manually. The most common error was
no-undef (eslint) which basically means that a variable has no definition in a file. I was able to fix that type of errors by simply adding:
/* import-globals-from ./util.js */
no-undef made up most of ESLint complaints. When I fixed all of them, I had a little bit more than 100 errors left. Here are some kinds of them:
Fix: Just remove the unused variable
This error points that a variable has the same name as another variable from an upper scope.
Fix: Rename the shadowing variable. Another way to fix it, which worked for me in many cases, is to use the value of the shadowing variable and use it directly wherever you use it.
Fix: Either remove eval or add to disable an eslint rule for it:
// eslint-disable-next-line no-eval.
constfrom the redeclared variable.
This error either points that the function should return something or it shouldn’t return a value.
Fix: Add or fix return of the function depending on an error message.
After I fixed all errors, I ran tests to make sure I didn’t break anything:
./mach mochitest dom/crypto
All of the tests passed ✅
Then, I committed my changes:
hg commit -m "Bug 1508825 Enable ESLint for dom/crypto (manual changes)"
Next step was to submit my fixes for review. It was not the most straightforward process for me because I used to run
git push origin master, and then create a pull request on GitHub. Here, I had to send my code for a review to Mozilla’s Phabricator instance. I followed Mozilla Phabricator User Guide to submit my patch:
Arcanist is the command-line interface to Phabricator, mainly used to submit patches for review. There is an Arcanist…moz-conduit.readthedocs.io
Linux and MacOS
Download moz-phab from the latest release and place it on your system path.
You must have Python 2.7 installed, and preferably in your path.
When you go to the latest release, you see:
I didn’t really know what I had to download. Should I download source code or just moz-phab file? I know that some of my classmates also found it confusing.
Luckily, I was able to guess it correctly, and I downloaded the source code. Then, I unarchived it. Next, I set PATH environmental variable to it, and I was able to run
moz-phab command (consider that I’ve already installed Arcanist).
Finally, I ran:
Both of my commits were sent for a review:
klymenko added a child revision: D13694: Bug 1508825 - Enable ESLint for dom/crypto (manual changes).Mon, Dec 3, 9:10…phabricator.services.mozilla.com
Volodymyr, Thank you for the patch, this is looking good.. There's a few changes here, but I think most of them are…phabricator.services.mozilla.com
I got a new experience and learned some new experience working on the large project such as Firefox. There were many new things for me behind editing and writing code in this contribution. Bugzilla, Mercurial, Arcanist, Phabricator, all these pieces of software were new for me, and I learned how to set up and use them. Even though I just worked with one folder, I was curious to see the project’s structure and organization, and it’s impressing.