Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
only reset the location on provisional load failure
Browse files Browse the repository at this point in the history
fix #3982
auditors: @bbondy @diracdeltas
  • Loading branch information
bridiver committed Sep 14, 2016
1 parent 994cb9f commit d348ff9
Showing 1 changed file with 5 additions and 9 deletions.
14 changes: 5 additions & 9 deletions js/components/frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const debounce = require('../lib/debounce.js')
const getSetting = require('../settings').getSetting
const config = require('../constants/config')
const settings = require('../constants/settings')
const { aboutUrls, isSourceAboutUrl, isTargetAboutUrl, getSourceAboutUrl, getTargetAboutUrl, getBaseUrl, isNavigatableAboutPage } = require('../lib/appUrlUtil')
const { aboutUrls, isSourceAboutUrl, isTargetAboutUrl, getTargetAboutUrl, getBaseUrl, isNavigatableAboutPage } = require('../lib/appUrlUtil')
const { isFrameError } = require('../lib/errorUtil')
const locale = require('../l10n')
const appConfig = require('../constants/appConfig')
Expand Down Expand Up @@ -825,7 +825,7 @@ class Frame extends ImmutableComponent {
interceptFlash(false)
}
}
const loadFail = (e) => {
const loadFail = (e, provisionLoadFailure = false) => {
if (isFrameError(e.errorCode)) {
// temporary workaround for https://github.com/brave/browser-laptop/issues/1817
if (e.validatedURL === aboutUrls.get('about:newtab') ||
Expand All @@ -851,12 +851,8 @@ class Frame extends ImmutableComponent {
})
windowActions.loadUrl(this.frame, 'about:error')
appActions.removeSite(siteUtil.getDetailFromFrame(this.frame))
} else {
const currentLocation = this.webview.getURL()
if (currentLocation !== e.validatedURL) {
windowActions.setUrl(isTargetAboutUrl(currentLocation)
? getSourceAboutUrl(currentLocation) : currentLocation, this.props.frameKey)
}
} else if (provisionLoadFailure) {
windowActions.setNavigated(this.webview.getURL(), this.props.frameKey, true)

This comment has been minimized.

Copy link
@diracdeltas

diracdeltas Sep 14, 2016

Member

this doesn't change the security state of the page to that of the old URL. ex: go to https://bing.com, load http://bayden.com/test/redir/goscript.aspx, the URL bar shows https://bing.com but with an unlocked icon. otherwise lgtm.

This comment has been minimized.

Copy link
@bridiver

bridiver Sep 14, 2016

Author Collaborator

hmmm... not sure what the best way to fix that is. Maybe we're clearing the security state too early? The other method worked because it reloaded url, but that also caused an infinite loop of reloads for some pages

This comment has been minimized.

Copy link
@diracdeltas

diracdeltas Sep 14, 2016

Member

opened #3988 for now

}
}
this.webview.addEventListener('load-commit', (e) => {
Expand Down Expand Up @@ -901,7 +897,7 @@ class Frame extends ImmutableComponent {
this.webview.addEventListener('did-fail-provisional-load', (e) => {
if (e.isMainFrame) {
loadEnd(false)
loadFail(e)
loadFail(e, true)
}
})
this.webview.addEventListener('did-fail-load', (e) => {
Expand Down

0 comments on commit d348ff9

Please sign in to comment.