This post continues the story I began in I contributed to Firefox. Here I will discuss the first bug I worked on, which became the first patch I landed.
The Bug
The first bug I took on was Bug 1895394 - Remove dom.event.wheel-event-groups.enabled pref.
The bug contained sparse instructions. There was the title, “Remove dom.event.wheel-event-groups.enabled pref” and a single comment from the bug reporter:
It’s been enabled by default for about a year now (bug 1823700). Let’s remove this feature rollout pref
At this point I didn’t know what a pref was or what it would take to remove it.
Finding and understanding the relevant code
Initial Leads
There were a couple leads to follow: the pref name and the bug mentioned in the comment quoted above.
The text dom.event.wheel-event-groups.enabled in the title looked like code that might match exact text in a search of the codebase.
The bug mentioned in the comment was for enabling the pref by default in the main release rather than only on the nightly developer release. It was resolved by a patch changing a single line, in a file named modules/libpref/init/StaticPrefList.yaml.
Near the modified line, I found that string, dom.event.wheel-event-groups.enabled. Evidently, this was where the pref was defined and given a default value, true in this case.
Text search for the above string yielded results in other files. These included test configurations where the pref was forced to a certain value for test execution. It also occurred in comments across several files, in the context of stating that something would be the case if the pref was on or off.
Confusingly, this initial search did not yield any location where the pref was being read. So how was its value being used?
Moving beyond exact text search
The code matching an exact text search did not reveal how the pref was being read. To figure that out, I had to search for context in the surrounding code at a more zoomed out scale.
StaticPrefList.yaml is a very long file, but at the top it does contain some documentation in the form of explanatory comments. This yielded an answer, explaining getter functions that can be made available for prefs:
# The getter function's base name is the same as the pref's name, but with
# '.' or '-' chars converted to '_', to make a valid identifier. For example,
# the getter for `foo.bar_baz` is `foo_bar_baz()`. This is ugly but clear,
# and you can search for both the pref name and the getter using the regexp
# /foo.bar.baz/.
Aha! A search using the above regex formatting, dom.event.wheel.event.groups.enabled (- replaced with .) yielded what I was missing: the sites where the pref is accessed.
Zooming out again to find documentation
The bug description made it clear that the pref I was removing was a feature toggle. That was enough to work on the bug, but I was itching for a clearer general understanding of what a pref could be.
The comments at the top of StaticPrefList.yaml don’t really explain what a pref is in general, speaking of the specifics of static prefs. However, the presence of those detailed comments spoke to the level of documentation embedded in the Firefox code.
Hoping to find something to explain prefs in general, I poked around up the directory tree and found that the libpref module contained a folder named docs with a single index.md. Bingo!
That document began by summarising that prefs can be used as feature toggles, preferences, debugging flags, and more.
With an understanding of generally what prefs are, and specifically where the one at issue was defined and used, I was ready to begin a solution.
Removing the pref
Removing the pref involved a few tasks:
- removing its definition
- removing value overrides (as in test configs)
- removing any mentions of the pref in comments
- refactoring consumers to behave properly without accessing the pref value
Most changes were simple
Removing the definition was as simple as deleting that block of code.
In all cases where the pref was being forced to a value for a test, it was being set to true. Since this was becoming not just the default but the only behavior, these tests would continue to work the same way with the pref removed, so I was able to simply delete the code setting the pref to true for these tests.
Comments mentioning the pref would no longer make sense once it was removed. This required individual attention to each context to ensure the comments made sense and were properly formatted after mention of the pref was removed.
Refactoring consumers
Deleting use of the pref required some attention that the remaining code was left in the appropriate state given what you are trying to make permanent.
I’ll give an example in pseudocode. Let’s say this is where a pref is accessed:
if (some_pref() && other_condition) {
// do stuff
}
If some_pref has been defaulted to true and this is being made permanent, the conditional simplifies to true && other_condition, which can be left as simply other_condition. if, however, it should be false, the conditional simplifies to false && ... which will never be true, and you would want to delete this entire conditional branch as it could never happen!
This pref was effectively being set true permanently, so I was able to take the former approach, effectively replacing any usage with true and simplifying accordingly.
In my patch I changed this:
if (StaticPrefs::dom_event_wheel_event_groups_enabled() &&
(wheelEvent->mDeltaX != 0.0 || wheelEvent->mDeltaY != 0.0)) {
To this:
if (wheelEvent->mDeltaX != 0.0 || wheelEvent->mDeltaY != 0.0) {
Finalizing the patch
I locally ran affected tests to verify they continued to pass. After checking with the bug reporter if I should run any other tests, I submitted the patch for review and it was approved. After spending a month in the beta, it shipped to Release channel Firefox with Firefox 150.
Practices and strategies that helped me
I’ll finish off by rambling about some things I did that seemed to be pretty helpful. These aided my first contributions to Firefox, but none are specific to that context.
Creatively explore the context
You can get a lot of mileage out of simple text search, especially with some creative use of regex or partial strings (e.g. wheel.event.groups instead of the whole thing). Search for file names, search within files… Perhaps use a bespoke search tool such as Searchfox if it exists for what you’re working on.
Look for explanatory comments or documentation. Check the top of long files, check parent directories, check external documentation sites. Click through if your bug mentions another bug, to see if it offers helpful context. (Reminder: In Firefox development, “bug” is semi-synonymous with “issue” or “ticket”.)
Coding agents such as Claude Code can be a helpful option for a fuzzy search of a codebase, especially if you aren’t sure what to run exact text search for. Usual cautions apply: double check conclusions, etc.
Take notes as you get up to speed
I take notes in markdown, linking related files with [[Wikilinks]]. I’ve heard good things about Obsidian. For accessibility reasons I use VSCode, with an extension project called Foam.
Save relevant links: to the bug page, related bugs, documentation…
Paste in a list of the files you modified, and use that to help figure out what tests you should be running. Save a code block with the command you need to run your tests. Record which tests are passing or not.
Explain in your own words what’s going on.
Draft questions to ask a bug mentor or other developer, and record key parts of their answers.
I took copious notes. They were helpful to remember what I had tried, what I had figured out or was still unsure about, and where key information was. They also made writing these posts much easier.
Final Thoughts
This was a good first bug. As I discuss at the end of I Contributed to Firefox, it’s easier to get used to the development process if the particular issue you are solving isn’t a huge lift.
I went on to land a couple more patches with similar scope, and as of writing I have a fourth patch in review which is quite different.
I have enjoyed my contributions to Firefox, and intend to make more.