Firefox 3 proxy auto config (PAC) reload button does not work

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.

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.

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.

Created attachment 318315
Proposed patch

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

Comment on attachment 318315
Proposed patch

can you instead add an argument forceReload to ConfigureFromPAC?

Created attachment 318657
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 on attachment 318657
updated patch

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

Comment on attachment 318657
updated patch

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

Comment on attachment 318657
updated patch

a1.9=beltzner

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

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

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

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.