DPS909 Release 03: My Second Open Source Contribution

So after a lengthy but rewarding experience with the completion of my pull request for my first open source contribution, we were now faced with building on this experience by tackling on Release0.3, which involved taking a step up from what we had already accomplished. To do this, I've decided to take on what I considered to be a more difficult bug for the brave browser. 

The bug i've chosen can be found here: https://github.com/brave/browser-laptop/issues/7645

Introduction; Explaining the Bug, Exploring Known Territory


The bug this time around involves the back and forward navigation buttons, more precisely, what happens when you press and hold one of those buttons for a long time. Here's a visual representation of the bug;


When pressing on the navigation buttons and holding the mouse click button, a dropdown list appears with the list of previously visited websites if you clicked and held back, and a list of previously navigated websites if you clicked and held forward. The bug happens when you release the click, and then click the same button again once to perform either the forward or back action. This will accomplish the action, but will keep the dropdown list alive and displayed on your screen, which is un-intuitive given that the list doesn't update after the second action of you clicking back or forward is performed. Therefore, this is a bug, and needs to be fixed.

This became familiar territory as I recalled my previous experience with Release0.2 and dealing with the Brave browser history and its components. This was a huge help at first as it gave me many leads on finding the source of this particular problem. Without too much effort, i've narrowed down the interesting parts of code involving these controls to these following files; 

app\common\state\contextMenuState.js
app\renderer\components\navigation\buttons\backButton.js
app\renderer\reducers\contextMenuReducer.js

Following this is a look into what's interesting in each of those files, including what i've added or modified.

Problem Solving Approach; How to Fix this Bug

The first thing i've done when approaching the problem was understanding how to fix it. In my release0.2, I had not been thorough enough in doing this step. I've looked into other browsers and tried replicating the bug in both Chrome and FireFox. They both did what I expected the behavior should be; when the menu shows up following a long press of the back or forward button, you can click the back or forward button again a single time, and the dropdown list goes away. This became the goal I was aiming for.

I've looked into the call stack of the events that happen when reproducing the bug, and that's what allowed me to narrow down the list of important files to the list mentioned above. Without further ado, here's a more in-depth look at each file;

File: app\common\state\contextMenuState.js


Now, the GitHub issue for this bug includes a comment that mentions a previous fix to a different issue was similar to what needs to be done for this problem. That fix included dealing with the navigation on the hamburger menu of the browser, which is included in the picture linked here. Generally speaking, this part of the code helps to set the context menu details in the case of the hamburger menu, so that when hovering with the mouse over any of the components of the menu, the menu knows to switch from one component to the other. Since we're looking for similar behavior here with the back and forward button, (with the only difference being in the way it is toggled), I have added the similar pieces of code with a new typing variable, (onBackLongPressMenuWasOpen), to indicate whether or not the menu should be displayed below the forward and back buttons.

File: app\renderer\components\navigation\buttons\backButton.js



This file was interesting because it deals with the class that constitutes the back and forward buttons. The methods that are called and should be needed for this fix are within this class as well. If you notice, the onBack method checks with multiple checks whether the previous visited tab is navigable or not. If it is, it clones the tab, renders it active. If not, it makes the current tab active which to the eyes of the user, does nothing. Now the onBackLongPress finds the parent node which helps it identify which component includes the dropdown list or whatever child component it may have. Storing it in the rect variable, it then passes its position coordinates to the onGoBackLong method which handles the action of displaying the menu. This particular method is a part of the next file we will be looking at;

File: app\renderer\reducers\contextMenuReducer.js




For the sake of explanation, this onLongBackHistory and onLongForwardHistory both have this similar structure, the only difference being how they retrieve history objects. Now, an outer if check was added with a type tag attached to the action, to try and prevent the creation of the submenu which happens within the second-most outer if check (line 541).

Once the code detects that there is a history to be shown by the submenu we know of, it creates a menuTemplate which will be sent somewhere else for displaying purposes. This is the part of the code we want to mess around with since we don't always want the menu to be created. To check out overwriting possibilities with the menu, the addition of the tag and detection of the tag within this code prevents the creation of the menu, therefore defaulting it to an empty template which you can find in the else statement on line 587. 

If you notice, line 584 includes the type being set to the typing we declared in the first file mentioned above. This is the tag that allows the setting of the contextmenu to know the difference. On the other hand, when we don't fall into this situation from the if check, line 590 takes care of toggling the type from what we've declared to a 'false', therefore allowing the creation of the submenu once the code hits this method once more.

Examining more Files, and Discussing the Solution

File: app\browser\reducers\tabsReducer.js


When it comes to this file, the APP_ON_GO_BACK and APP_ON_GO_BACK_LONG as well as their FORWARD counterparts are the sections of code being called and methods being deployed once the handling of the back and forward submenus come into play. This file is important because the way the information gets sent to the goBack and onLongBackHistory methods from the files seen above can dictate how the display of these components should be handled before it gets there. 

Discussing the Solution

The solution has not been completed as of now (2018-04-22); there are key steps missing in these files, however the locations where logic needs to be added have been pinpointed as of now. Generally speaking, if the toggling mechanism works and lets the code know how to initialize the components under the forward and back buttons, the rest will follow.

Closing Words

This will be my last post in regards to my Open Source Development class, and as such, I would like to take this chance to speak about how valuable an experience this has been. As a programmer, having had the chance to touch so many concepts of open source development within the short timeframe that is a semester, it has simply been very enriching. 

In regards to our teacher, David Humphrey, we've had a very knowledgeable information bank at our disposal as the course progressed, and will remain one of the important factors that let me expand my knowledge of Open Source development.

In regards to our release assignments which often included pull requests and contributions to open source projects as opposed to our laboratory experiments, they will continue to be hard fact proofs of all of our progress in the class within the open source world. Being able to say we've contributed to major projects is an achievement that will last a long time.

Finally, in regards to the classmates and the way the course was built, the experienced was enhanced further in the form of contributing to our own projects as a base to build onto. These structures are what led us to the possibility of contributing to major projects, and eased our way there as much as they should.

I hope to continue blogging someday about programming as it is a passion and career, and a good reminder and help to others who share that as a career.

Comments

Popular posts from this blog

Running Test Suites and playing with ECMA Standards

DPS909 Release 02: My First Open Source Contribution Part 1

Finding and fixing a bug in VSCode