Bug fix in Firefox Focus for iOS

I continue my journey in the world of Open Source Software, and I would like to share my recent experience of fixing a bug in a large iOS application. This application is Firefox Focus for iOS.

This contribution is notable to me because it was my first contribution where I fixed a real bug in such a large project. My previous contributions mainly were focused on code/project enhancements.

If you haven’t read my story about my participation in Hacktoberfest 2018, you can find it here:

Now, let me tell you about the project and bugfix.

Firefox Focus 🔥🦊

Firefox Focus for iOS devices is a privacy browser with tracking protection and content blocking. (Retrieved from Mozilla)

You can download Firefox Focus on App Store.

Also, if you are into iOS development, or you are into open source development, you can contribute to this project on GitHub.

Browse like no one’s watching. The new Firefox Focus automatically blocks a wide range of online trackers — from the moment you launch it to the second you leave it. Easily erase your history, passwords and cookies, so you won’t get followed by things like unwanted ads. (Retrieved from app’s description on App Store)

Here are a few screenshots of this app:

Firefox Focus for iOS
“Private browsing” on most browsers isn’t comprehensive or easy to use. Focus is next-level privacy that’s free, always on and always on your side — because it’s backed by Mozilla, the non-profit that fights for your rights on the Web. (Retrieved from app’s description on App Store)

One of its feature is that if you don’t want to switch from Safari, you can still use Firefox Focus functionality in Safari (follow these steps to do that).

Bug ❌

The issue was that the search text in URL Bar mismatched the actual search after using back and forward navigation. Let’s go through an example:

  1. Search “the guardian”
  2. Search “mozilla”
  3. Press back
  4. Search “firefox”
  5. Press back
  6. Search page shows results for “the guardian” request, but URL Bar contains “mozilla” instead of “the guardian”.

Fix 🔧

The author of the issue left a note there that the problem might be in SearchHistoryUtils stack. It helped me to start my investigation of the problem.

I found class SearchHistoryUtils, which contains two boolean properties.

  1. isFromURLBar
  2. isNavigating

and four functions:

  1. pushSearchToStack(with searchedText: String) — when user makes a new search request, this function pushes that search to the stack.
  2. pullSearchFromStack() -> String? — this function doesn’t pop search from stack, instead, it returns the value of current search text if present.
  3. goForward() — updates isCurrentSearch value for the stack’s objects.
  4. goBack() — same as #3

When I was analyzing the code, I dismissed pullSearchFromStack, goForward, and goBack functions because they didn’t really deal with stack changes. Therefore, I started debugging pushSearchToStack function. I debugged several test cases, and let’s take a look at my findings.

Reflections of stack value on user actions, where bold font shows change caused by user action, and the red line is the place which causes a bug.

So I pretty much reproduced the bug described in the issue, and I found the problem:

When the last search is not the current search (like in the Stack after first “Go back” user action), and user makes a new search, the last search is not overridden with the new one (look at red line in the table above).
That is why when user goes back, URL Bar takes the old search text, which shouldn’t even be in the stack at that moment.

Let’s take a look at a chunk of the pseudo code of pushSearchToStack function:

A chunk of the pseudo code of pushSearchToStack function.

Let’s take apart the code above:

  1. It declares an empty stack currentStack.
  2. It sets the value of currentStack to the value of global state searchedHistory value.
  3. Then, it goes through each search in the stack, and sets value of isCurrentSearch property to false.
  4. It adds the new search to the stack and sets its isCurrentSearch property to true.

So I played with the code a bit, and added a check whether the last search is the current search:

A chunk of the pseudo code of pushSearchToStack function.

If the last search is not the current search, it means that user clicked “back” button, and the currentStack removes its last element.

I tried to simulate the test case described above, and it worked as it supposed to work! The search text in URL Bar matched the search page.

However, it was not the final fix. This approach doesn’t work in case user goes back more than one time because the code above removes only last search from the stack.

Even though it wasn’t right, it let me understand that I am on the right way.

So I evolved my code:

A chunk of the pseudo code of pushSearchToStack function.

Now, it checks whether the last search is the current search. If it is not the current search, it looks for the currentSearch in the stack and removes all the following searches from the stack.

Then, it adds a new search with the value of isCurrentSearch property to true to the stack.

The whole contribution you can find in my Pull Request.

Finally, let’s take a look at the fixed version of the browser.

As you can see, the text in URL Bar matches the search page even if you go back and forward as many times as you want.

Conclusion

My pull request was successfully merged ✅

Overall, I liked the experience of contributing to this project, and I am likely to work on other issues there again 👨🏻‍💻🔜🔥🦊