Closed Bug 422172 Opened 12 years ago Closed 12 years ago

"Automatic proxy configuration URL" reload button does not work

Categories

(Core :: Networking, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla1.9

People

(Reporter: christopher+ffoxbugzilla, Assigned: hugues.fournier)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4) Gecko/2008030714 Firefox/3.0b4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4) Gecko/2008030714 Firefox/3.0b4

If I start firefox on network A, the automatic proxy configuration loads the proxy settings correctly, and I am able to browse. If I then move the computer to network B, on which the auto proxy config server will return a different proxy server, I am not able to browse, as the proxy is not found. (I get "The host X is taking to long to respond"). Pressing the Reload button under Tools - Options - Advanced - Network - Connection [Settings] does not reload the proxy configuration. I have to restart firefox for the new proxy settings to work.

An additional request to the above bug would be to have Firefox automatically reload the proxy on network change (as for instance IE does).

Reproducible: Always

Steps to Reproduce:
1. Define 2 networks
2. Set automatic proxy URL to URL that returns "direct" on network A and "w.x.y.z" on network B.
3. Start firefox on network A to load proxy "direct"
4. Move computer to network B and try to reload proxy settings
Actual Results:  
All browsing returns either

"Site X takes to long to respond" (if I have the "direct" settings loaded on the proxy-requiring network)

OR

"Firefox is set to use a proxy that does not exist" if I started firefox on net with proxy and then moved to a non-proxy network.

Expected Results:  
Preferrably: Automatically reloaded the proxy information.
At least: Reloaded the proxy settings upon manually hitting the "Automatic proxy configuration URL" reload button.
Also doesn't work on
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b4) Gecko/2008030318 Firefox/3.0b4

The same configuration file worked on FF2. Restarting FF3b4 loads file correctly.

Config file:

function FindProxyForURL(url, host) {
    var internal_proxy = 'DIRECT';
    var no_proxy = 'DIRECT';
    var external_proxy = 'PROXY xx.xx.xx.xx:8888';

    var exclude_proxy = 'PROXY 127.0.0.1:80';

    if (shExpMatch(host, "*.draugiem.lv")) {
      return external_proxy;
    } else if (shExpMatch(url, "*draugiem.lv*")) {
      return external_proxy;
    } else if (shExpMatch(url, "*inbox.lv*")) {
      return external_proxy;

   /* repeats for lot of hosts */

   } else if (host=="neo") {
      return no_proxy;
    } else if (shExpMatch(url,"http*://*:*/*")) {
      return no_proxy;


    } else if (shExpMatch(url,"https://*")) {
      return internal_proxy;
    } else {
      return no_proxy;
    }
}
Confirming this based on this filing as well as reports in hendrix.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: unspecified → Trunk
Christopher, do you think you could find a 1-day regression range for the behavior changed you see between Firefox 2 and Firefox 3 beta 4, using -trunk builds from http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/ ? It would help a lot.
Component: Preferences → Networking
Product: Firefox → Core
QA Contact: preferences → networking
Gavin, I will try to locate the checkins that introduced this regression these coming days..
I have located the checkin that introduced this regression.

The regression appeared between build 2008-01-29-04-trunk and build 2008-01-29-04-trunk
The probable suspect seemed to be the checkin that resolved bug 66057 :
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/base/src/nsProtocolProxyService.cpp&rev=1.74&root=/cvsroot
(that had been checked in, then backed out, then relanded on January 29)

And it was..

Since this checkin, ConfigureFromPAC exits if the uri is the same than the one already known, this is an intended behaviour:
"Switch to new PAC file if that setting has changed. If the setting hasn't changed, ConfigureFromPAC will exit early." (comment in the new code of Resolve_Internal introduced in this checkin)

The problem is that ConfigureFromPAC is called from ReloadPAC() too.

Attached patch Proposed patch (obsolete) — Splinter Review
Here is a proposed patch allowing to short-circuit the "early exit" if the reload is forced.
Attachment #318315 - Flags: review?(cbiesinger)
Comment on attachment 318315 [details] [diff] [review]
Proposed patch

can you instead add an argument forceReload to ConfigureFromPAC?
Attachment #318315 - Flags: review?(cbiesinger) → review-
Attached patch updated patchSplinter Review
I had avoided this way of doing because, for whatever stupid thoughtlessness ou tiredness, I had wrongly seen that ConfigureFromPAC was exposed to XPCOM (I had probably confounded it with ReloadPAC)..
Attachment #318315 - Attachment is obsolete: true
Attachment #318657 - Flags: review?(cbiesinger)
Attachment #318657 - Attachment description: update patch → updated patch
Attachment #318657 - Flags: review?(cbiesinger) → review+
Attachment #318657 - Flags: superreview?(darin.moz)
Comment on attachment 318657 [details] [diff] [review]
updated patch

biesi can sr this as well (and presumably it is implied by his r+).
Attachment #318657 - Flags: superreview?(darin.moz) → superreview?(cbiesinger)
Attachment #318657 - Flags: superreview?(cbiesinger) → superreview+
OS: Windows XP → All
Hardware: PC → All
Assignee: nobody → hugues.fournier
Flags: wanted1.9.0.x?
Comment on attachment 318657 [details] [diff] [review]
updated patch

We shouldn't ship broken UI. The patch is very safe.
Attachment #318657 - Flags: approval1.9?
Comment on attachment 318657 [details] [diff] [review]
updated patch

a1.9=beltzner
Attachment #318657 - Flags: approval1.9? → approval1.9+
Checking in netwerk/base/src/nsProtocolProxyService.cpp;
/cvsroot/mozilla/netwerk/base/src/nsProtocolProxyService.cpp,v  <--  nsProtocolProxyService.cpp
new revision: 1.82; previous revision: 1.81
done
Checking in netwerk/base/src/nsProtocolProxyService.h;
/cvsroot/mozilla/netwerk/base/src/nsProtocolProxyService.h,v  <--  nsProtocolProxyService.h
new revision: 1.34; previous revision: 1.33
done
Status: NEW → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9
I backed this out to see if it was causing crashes, but it wasn't the patch. I'll reland this later today.
Status: RESOLVED → REOPENED
Keywords: checkin-needed
Resolution: FIXED → ---
Relanded.

Checking in netwerk/base/src/nsProtocolProxyService.cpp;
/cvsroot/mozilla/netwerk/base/src/nsProtocolProxyService.cpp,v  <--  nsProtocolProxyService.cpp
new revision: 1.84; previous revision: 1.83
done
Checking in netwerk/base/src/nsProtocolProxyService.h;
/cvsroot/mozilla/netwerk/base/src/nsProtocolProxyService.h,v  <--  nsProtocolProxyService.h
new revision: 1.36; previous revision: 1.35
done
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Flags: wanted1.9.0.x?
V/fixed.

I recently unit-tested the reload button's behavior to make sure it pulls from the http server. I used my test http server's access log to confirm.

Mac OS X 10.5, FF 3.0.1.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.