Thursday, October 25, 2018

Hacktober IV

This is my fourth blog post in my Hacktober Series. Throughout these 5 blog posts I hope to explain my introductory work into contributing to Open Source Software.

What I Worked On

I went back to the focus iOS browser for my fourth pull request. This time around, I decided to tackle an issue where pressing on the settings button would break the layout for the web view. The reason I chose this bug in particular is because of the needs investigating label attached to the issue. I figured it'd be a great issue to try to fix in order to continue learning about the codebase. To fix this problem, I had to investigate the cause for the broken layout.



The Fix

The fix itself was easy. Just like in my second commit, this fix was a line long.



The real challenge behind this pull request was trying to figure out what was causing the problem. After some digging around, I found out that the code was setting urlBar.shouldPresent to false when opening the settings popup (see code above). This caused the urlBar object to never call any of its delegate methods to active. Since it never activates, its layout remained deactivated as well. The webview bases its bounds based on the bottom of the urlBar - since the urlBar's overwritten constraints were never turned back on by the delegate methods, the webview used the urlBar's default constraints to base its own bounds on. This is what created the layout issue.

After looking through older commits for the functionality, I found that the reason that urlBar.shouldPresent was originally set to false was to prevent the application from displaying the overlay when launching from an extension. After failing to be able to reproduce this, I decided to ask the community and received a reply pointing me to the right direction.

After checking the history on git, I realized that the settings view controller was not originally presented modally and that's why the urlBar was turned of when viewing the settings. Since the change, the bug that setting urlBar.shouldPresent to false was supposed to fix was no longer possible to reproduce - instead of a fix, it itself became the cause of a new bug - so it was safe to delete.

My Pull Requests

No comments:

Post a Comment