422172 - "Automatic proxy configuration URL" reload button does not work

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[Christopher Pharo Glæserud]

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.

Comment 1

[Atis Lezdins]

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;
    }
}

Comment 2

[Marcia Knous [:marcia]]

Confirming this based on this filing as well as reports in hendrix.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

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.

Comment 4

[Hugues Fournier (french l10n team)]

Gavin, I will try to locate the checkins that introduced this regression these coming days..

Comment 5

[Hugues Fournier (french l10n team)]

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.

Comment 6

[Hugues Fournier (french l10n team)]

Attachment: Proposed patch

Here is a proposed patch allowing to short-circuit the "early exit" if the reload is forced.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 318315 [details] [diff] [review]
Proposed patch

can you instead add an argument forceReload to ConfigureFromPAC?

Comment 8

[Hugues Fournier (french l10n team)]

Attachment: updated patch

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)..

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 318657 [details] [diff] [review]
updated patch

biesi can sr this as well (and presumably it is implied by his r+).

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 318657 [details] [diff] [review]
updated patch

We shouldn't ship broken UI. The patch is very safe.

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 318657 [details] [diff] [review]
updated patch

a1.9=beltzner

Comment 12

[Reed Loden [:reed]]

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

Comment 13

[Reed Loden [:reed]]

I backed this out to see if it was causing crashes, but it wasn't the patch. I'll reland this later today.

Comment 14

[Reed Loden [:reed]]

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

Comment 15

[benc]

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.