Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in

Issue 29350379: Issue 4386 - Update frame URL after redirection (Closed)

Can't Edit
Can't Publish+Mail
Start Review
3 years, 9 months ago by kzar
3 years, 8 months ago
Wladimir Palant
Sebastian Noack


Issue 4386 - Update frame URL after redirection

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -0 lines) Patch
M chrome/ext/background.js View 1 chunk +9 lines, -0 lines 1 comment Download


Total messages: 3
Patch Set 1
3 years, 9 months ago (2016-09-01 13:38:16 UTC) #1
https://codereview.adblockplus.org/29350379/diff/29350380/chrome/ext/background.js File chrome/ext/background.js (right): https://codereview.adblockplus.org/29350379/diff/29350380/chrome/ext/background.js#newcode151 chrome/ext/background.js:151: { (We could also check that details.transitionQualifiers is ["server_redirect"] ...
3 years, 9 months ago (2016-09-01 13:42:07 UTC) #2
Wladimir Palant
3 years, 9 months ago (2016-09-05 15:37:40 UTC) #3

The issue isn't limited to redirects. Just updating the URL in onCommitted
doesn't work - the approach used right now is flawed, setting URL in
onBeforeNavigate is just bad. Here is how you can see it:

1. Open JS Console on example.com.
2. Run `location.href = "http://google.com:1234/";` (server isn't listening on
this port so "loading" will spin forever).
3. Now press Esc to abort loading.
4. Run `new Image().src="http://test/";` and check "Adblock Plus" panel to see
which host this request was attributed to.

Currently Adblock Plus will set document URL to "google.com" even though that
load never happened. Consequently, any subsequent loads will be attributed to
Google and not the real site.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5