Update background.js (fixes #12377) #12380
Conversation
ghost
mentioned this pull request
|
@koops76 the variable is being used again in |
ghost
mentioned this pull request
|
@cschanaj Already done. |
chromium/background.js
Outdated
| @@ -384,7 +386,7 @@ function onBeforeRequest(details) { | |||
| // failing. | |||
| if (shouldCancel) { | |||
| if (!newuristr) { | |||
| return {redirectUrl: canonical_url.replace(/^http:/, "https:")}; | |||
| return {redirectUrl: details.url.replace(/^http:/, "https:")}; | |||
There was a problem hiding this comment.
IMHO, this is change is not necessary, better not to change the existing behavior.
There was a problem hiding this comment.
This is actually why they are replaced with null but not replaced back.
There was a problem hiding this comment.
@cschanaj Even better now, bug is fixed but the existing behavior is kept.
| @@ -320,12 +320,14 @@ function onBeforeRequest(details) { | |||
| // If there is a username / password, put them aside during the ruleset | |||
| // analysis process | |||
| var using_credentials_in_url = false; | |||
| let tmp_user; | |||
There was a problem hiding this comment.
not even consistent in the same function ...
There was a problem hiding this comment.
Will be when we will run ESLint with new settings.
|
lot more changes than I have expected. I am not going to review this (simply not having the time, not against you). ping Hainish when you are done, thanks anyway. |
|
ping @Hainish. |
| const canonical_url_with_credentials = new URL(canonical_url); | ||
| canonical_url_with_credentials.username = tmp_user; | ||
| canonical_url_with_credentials.password = tmp_pass; | ||
| canonical_url = canonical_url_with_credentials.href; |
There was a problem hiding this comment.
It should be safe to add this as an else clause to the conditional immediately below. If newuristr is defined, this mutation of canonical_url is simply unused.
This is a minor point and shouldn't block merging, but it makes things a bit easier to follow with the matching if (!newuristr) below.
No description provided.