Link to Lab: https://wiki.cdot.senecacollege.ca/wiki/OSD600_and_DPS909_Winter_2018_Lab_6
In this lab we have to make tests for Brave and then fix them.
We are trying something called test driven development.
That means we make tests for features that fail and then we work to fix them.
We need to test the following queries in the URL bar:
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| "dog" | |
| " dog " | |
| "dog cat" | |
| " dog cat " | |
| "https://www.google.ca/search?q=dog" | |
| " https://www.google.ca/search?q=dog " | |
| "https://www.google.ca/search?q=dog cat" | |
| " https://www.google.ca/search?q=dog cat " |
Here are the test results I did in various browsers:
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| "dog" | |
| Chrome: Good | |
| Brave: Good | |
| Edge: Good | |
| IE: Good | |
| " dog " | |
| Chrome: Good | |
| Brave: Good | |
| Edge: Good | |
| IE: Good | |
| "dog cat" | |
| Chrome: Good | |
| Brave: Good | |
| Edge: Good | |
| IE: Good | |
| " dog cat " | |
| Chrome: Good | |
| Brave: Good | |
| Edge: Good | |
| IE: Good | |
| "https://www.google.ca/search?q=dog" | |
| Chrome: Good | |
| Brave: Good | |
| Edge: Good | |
| IE: Good | |
| " https://www.google.ca/search?q=dog " | |
| Chrome: Good | |
| Brave: Good | |
| Edge: Good | |
| IE: Good | |
| "https://www.google.ca/search?q=dog cat" | |
| Chrome: Good | |
| Brave: Good but a bug as it displays https://www.google.ca/search?q=dog cat in google search | |
| Edge: Good | |
| IE: Good | |
| " https://www.google.ca/search?q=dog cat " | |
| Chrome: Good | |
| Brave: Same as above | |
| Edge: Good | |
| IE: Good |
And I had my teammate run the same tests and he confirmed that the results are the same on the Mac.
I find it pretty bad that IE 11 beats out Brave when it comes to interpret the URLs.
In order to fix the problem I removed this code in urlutil.js:
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| //if (case1Reg.test(str)) { | |
| // return true | |
| //} | |
| //if (case2Reg.test(str) || !case3Reg.test(str) || | |
| // (scheme === undefined && /\s/g.test(str))) { | |
| // return true | |
| //} |
and added this code:
input = input.replace(/\s/,'%20')
I needed to remove that code because it was catching spaces in the URL and as a result the URL would be interpreted as a string. The second if statement was causing problems too. I needed to add the replace because I needed to convert ' ' to %20.
However, after talking with my teammate we decided that was probably not the best way of dealing with it. Instead we choose to modify case2 and added a case for dealing with windows. Looking at case2 regex it was done wrong.
Case2 originally looked like this: const case2Reg = /(^\?)|(\?.+\s)|(^\.)|(^[^.+]*[^/]*\.$)/
The documentation says it checks for “? “. However, this is not the case as the second capture group is incorrectly made.
It should look like this: const case2Reg = /(^\?)|(\?\s+)|(^\.)|(^[^.+]*[^/]*\.$)/
Looking at Brave’s code it didn’t convert ‘/\ to ‘/’ which lead to other problems. We needed an if statement that checked for line endings and convert them to unix style and prepended the correct scheme.
Additionally, we added this regex check: const case5Reg = /(?:^\/)|(?:^[a-zA-Z]:\\)/ which checked for the windows and unix file paths.
Also, we created a bunch of tests that tested isNotUrl() and getUrlFromInput().
Which we then used to make sure our fix worked. It failed at first but with the fixes we applied it passed all of the tests.
We identified a couple of strings that caused problems. First, https://www.google.ca/search?q=dog cat gets rendered as a string rather than a URL. This results in a URL looking like this: https://www.google.com/search?q=https%3A%2F%2Fwww.google.ca%2Fsearch%3Fq%3Ddog%20cat . This happens after hitting the enter button. Also, the way Brave interprets E:\files\dog cat.txt caused some problems with Brave because it is interpreted as a string which is not correct. All the other browsers could render the URL properly. On a side note I finally got VSCode debugging to work with Brave because of my teammates’ help.
I found it interesting that IE rendered E:\files\dog cat.txt as E:\files\dog cat.txt which is different than Chrome, Edge, IE which interpreted it as file:///E:/files/dog%20cat.txt.
Looking at Mozilla’s code they only get rid of trailing and leading whitespaces. They don’t get rid of all white spaces like Brave does. In addition, I don’t think that Chrome removes all white space and follows Mozilla.
In order for me to fix the issue I needed to learn how Brave’s tests worked and how Brave organized their code. It didn’t take too long as the URL testing file is broken down into separate sections based on methods and the URL file is not too huge. Also, I needed to brush up on my regex skills because I needed to read a lot of regex’s to find out which regex was causing the problem.
I needed to remove some of Brave’s checks in order for Brave to behave the same as the other browsers. In addition, there was no code in Brave that converted ' ' to %20 so I needed to implement it. Also, this was my first time working on a lab with a partner. My partner helped me complete this and it was useful working with someone else. We could bounce ideas off of each other and get help from each other.
This lab was a lot of work but in the end it was a rewarding experience.
Matt's Programming Blog 