Don't use the previous URL if current virtual URL is safebrowsing #10161
Conversation
app/browser/tabs.js
Outdated
| @@ -302,7 +302,8 @@ const fixDisplayURL = (navigationEntry, controller) => { | |||
| navigationEntry.virtualURL = getSourceAboutUrl(navigationEntry.virtualURL) | |||
| } | |||
|
|
|||
| if (isIntermediateAboutPage(navigationEntry.virtualURL)) { | |||
| if (isIntermediateAboutPage(navigationEntry.virtualURL) && | |||
| navigationEntry.virtualURL.indexOf('about:safebrowsing#') === -1) { | |||
There was a problem hiding this comment.
if this is an issue for about:safebrowsing, shouldn't it also be an issue for about:error, about:certerror, etc?
There was a problem hiding this comment.
maybe not because those errors fire after the page has started loading already but about:safebrowsing fires before
There was a problem hiding this comment.
i think this should do a more strict check, like !virtualUrl.startsWith...
There was a problem hiding this comment.
@diracdeltas I'm not sure, maybe you can help me understand
If you open a new tab and then nav to downloadme.org, filtering picks it up via https://github.com/brave/browser-laptop/blob/0.18.x/app/filtering.js#L150 and restarts the nav process with "about:safebrowsing#downloadme.org", causing the previous page to be the full chrome-extension://about-newtab.html. It's not a true interstitial anymore since there is no previous entry for http://downloadme.org
If you navigate to a site and turn off your network card while the connection is happening, I would expect the browser to have an error page... but have the proper URL in the url bar (ex: the site I'm trying to reach). I was under the impression that a redirect to about:error happens after the site is unreachable (so the previous entry would be the correct site)
There was a problem hiding this comment.
doh- missed your previous comments 😛 (needed to refresh). Let me implement a better check 😄
Codecov Report
@@ Coverage Diff @@
## master #10161 +/- ##
==========================================
- Coverage 52.81% 52.8% -0.01%
==========================================
Files 227 227
Lines 20195 20194 -1
Branches 3234 3234
==========================================
- Hits 10665 10663 -2
- Misses 9530 9531 +1
|
Fixes #10143 Auditors: @diracdeltas, @alexwykoff Test Plan: 1. open new tab 2. navigate to downloadme.org 3. notice that URL is properly "about:safebrowsing#downloadme.org"
|
@diracdeltas updated 😄 |
Don't use the previous URL if current virtual URL is safebrowsing
Don't use the previous URL if current virtual URL is safebrowsing
Fixes #10143
Auditors: @diracdeltas, @alexwykoff
Test Plan:
Submitter Checklist:
git rebase -ito squash commits (if needed).Reviewer Checklist:
Tests