79246 - PAC: reload of PAC file fails

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[[e-mail]]

From bug 53080:
------- Additional Comments From [e-mail] 2001-04-26 10:42 -------

For me, Linux 2001042608 nightly, there is at least one serious problem.

There was an error in my original PAC file, so I uploaded a new one.
Mozilla ignored the new version, even after pressing Reload in the proxy prefs
dialog and restarting.
I renamed the fixed file, changed the entry in prefs, hit reload, restarted
Mozilla multiple times. No effect.

If I can do any more to test this, or if anyone knows a way (short of blowing
away my profile or mozilla) to reload the PAC please mail me.
If PAC is officially "WORKING" and this needs to be a new bug, you have only to
say the word.

--
I have also observed this by trying to use the reload button and also the URL
field to send new requests for the PAC file. I snooped the port the PAC file
lives on, and verified that it was not being reloaded over the network.

Comment 1

[benc]

qa to me.

Comment 2

[benc]

CONFIRMED: mozilla 0.9 Win32

I've found a couple problems with PAC file loading, and this will probably need
to become several bugs:

1- The Reload button does not work. Tapping the reload button while tailing my
proxy log shows no activity.

2- PAC does try to load everytime you change the URL and hit "ok", but it does
not change the PAC file that is loaded into necko.

3- PAC will fail to send a request for a file that has been loaded before
(probably because it is in cache). Setting cache to "everytime" does not force a
new request.

Comment 3

[benc]

nominating nsbeta1

This should not be futured. This needs to work because one of the features of
PAC is that the network-based configuration is centrally administered and easily
updated.

Comment 4

[neeti]

-->PAC bugs to Jussi

Comment 5

[neeti]

-->PAC bugs to Jussi

Comment 6

[benc]

COMM4:

I've testd Communicator, and found that the reload-button does fire every time
you push it.

Comment 7

[Jussi-Pekka Mantere]

Serge, can you take a look at these PAC issues? Thanks - Jussi-Pekka

Comment 8

[Chase Tingley]

http://lxr.mozilla.org/seamonkey/source/xpfe/components/prefwindow/resources/content/pref-proxies.xul#141

It's unsurprising that this doesn't work, as the reload button doesn't have an
oncommand handler.

What this handler would do is another issue.  nsIProtocolProxyService doesn't
provide an interface to tell it to configure itself from a PAC.  Currently, all
the action is done by detecting a change in the network.proxy.autoconfig_url
preference.  Maybe there's some way to hack it.

Comment 9

[benc]

+mozilla0.9.2 - for what it's worth.
+relnote - I'll think of something by RTM.

Comment 10

[Chase Tingley]

Proposed solution: add an interface ConfigureFromPAC() to
nsIProtocolProxyService to force config from a specified URL.  Then the reload
button's oncommand handler has some way of indicating that a reload is needed;
detection of a change in the url field will trigger this internally as well.

I've got a sample implementation of this that seems promising -- I've tested it
against file:/// urls and verified that it works.  However, it's basically at
the mercy of the cache when it comes to retrieving PACs over http.  I need to
dig around some more and possibly chat with someone from networking about how to
get around this.

Comment 11

[Chase Tingley]

I found a way to bypass the cache when loading the PAC file, and I've tested out
the implementation against a real web server to verify that it reloads properly.  

Now I'm wondering about when we *should* load the PAC from cache.  In addition
to being called whenever "Reload" is clicked, ConfigureFromPAC() is being called
at startup when the proxy prefs are parsed.  In this case, is it better to load
from cache if available?  Is there any other time I would also want to load from
cache?

Comment 12

[Peter Murray]

If we want to emulate past behavior, I believe we should hit the server for 
the PAC file each time it is requested, whether when hitting the reload 
button or at startup.  If the PAC file can't be retrieved from the server, 
Mozilla should display a warning and use the PAC file from cache.

Past behavior or not, I think that is the best way to handle it, too.

Comment 13

[benc]

My gut says we should ignore cache. The reason I think so is that it is too easy
for an inexeperienced web admin to post a PAC that would cache badly. And if
people keep their PAC files a sane size, then server congestion is not a problem.

Comment 14

[Chase Tingley]

Attachment: proposed patch

Comment 15

[Chase Tingley]

Ok, there it is.  Old PrefsChanged() code to kick off the PAC load has been
moved into a separate api, which is also called from the onload handler. 
Caching is always bypassed.

If it's possible to get this into 0.9.3 in some form, I'd like to do it.

Comment 16

[benc]

If the PAC loading happens everytime you close prefs (change or no change), then
we could get rid of the reload button.

I dislike the reload button because there is no user feedback if it loaded or
failed (even in Communciator 4).

Comment 17

[Chase Tingley]

If the PAC were reloaded every time the prefs closed, that might be enough
(although opening the prefs and then closing them to force a reload is sort of a
weird interface design...).  Currently, it only reloads when the url changes.

Comment 18

[basic]

could we add user feedback to it? Maybe that should be another bug?

Comment 19

[Chase Tingley]

cc'ing powerful network people in hopes of getting an r/sr or a rejection, etc.
before I go on vacation (and out of email contact) next monday (7/30).

Thanks in advance.

Comment 20

[serge (gone)]

The patch looks fine for me, for feedback in case of loading failure we have to 
open new bug, I think.
r=serge

Comment 21

[Blake Ross]

Missed 0.9.3.

Comment 22

[benc]

RELNOTE for NS 6.1:

The "reload" button in Proxy Auto Config does not work. 
--
+qawanted
 
I think the likely workaround is to clear the caches and then close preferences
w/ "okay". I probably need to test this, unless someone wants to.

Comment 23

[Darin Fisher]

one minor nit:

-            this->ConfigureFromPAC(tempString);
+            ConfigureFromPAC(tempString);

remove the redundant |this->| and sr=darin.

Comment 24

[serge (gone)]

[e-mail] thank you
checked in
resolve as FIXED

Comment 25

[Pub.1]

"onLoad is not defined tabs.js:309" reload button for the proxy.pac Firefoxy 28.0 pt-br XP+SP3 PtB.

Comment 26

[Peter Bylenga [:PBylenga]]

Striking old qawanted tag, if something is needed please re-add.