41072 - Don't migrate auto-conf pref until this feature is turned on

Status: VERIFIED WONTFIX

(Core Graveyard :: Profile: Migration, defect, P1)

Description

[Chris McAfee]

Migrating auto-config=true pref will lock up the browser
since this feature isn't implemented yet.

Comment 1

[Chris McAfee]

This is related to bug 20145, the bug-holder for proxy auto-config feature.

Comment 2

[Tomi Leppikangas]

Proxy-autoconf isn't still in, this should be fixed before ns6. Adding depend
to 53080, which is bug for autoconfig work.

Comment 3

[(not reading, please use [e-mail] instead)]

adding gagan, me and racham to the cc list.

we should migrate the auto-config pref, but the back end should just ignore it
if it causes a hang.

I don't think this should be dbragg's bug, it should be gagans.

Comment 4

[Daniel Veditz [:dveditz]]

Reassigning, raising priority since this causes a hang

Comment 5

[(not reading, please use [e-mail] instead)]

on linux, I don't get the browser lock up, but I am unable to browse to any sites.

mcafee, is that what you mean by lockup?

Comment 6

[Chris McAfee]

I don't remember why I used "lockup", that sounds like
sspitzer asking me to file a bug to me.  I'll go with
what sspitzer says in the above comment.

Comment 7

[(not reading, please use [e-mail] instead)]

gagan, would it be better to treat the type as 0 if we read 2 from the prefs?

(network.proxy.type is the pref, 2 == use auto-conf.)

we'd leave the pref alone, but just act like it wasn't set.

see nsProtocolProxyService.cpp, line 109

Comment 8

[Gagan]

nope! that would break PAC from being used altogether. The safest way to disable
that may just be within the installer to not migrate the pref.

Comment 9

[dbragg]

Just to interject something here, the concept of "not migrating a pref" is
actually a matter of explicitly removing the pref or explicitly setting it to
something innocuous.  The migration code (for the most part) simply copies the
prefs.js file to the new profile.  The prefs are NOT read through and "migrated"
(except in a couple of mail/news cases). It's up to the various parts of the
program (browser, mail/news, AIM, whatever) to read the old prefs and either
ignore them, use them or update them where appropriate.  So you can see that not
migrating a pref really means reading the prefs.js file, seeing if the pref is
set and then changing it.

Then when the auto-config feature is implemented we have to remember to rip out
the code in the migration component that always turns it off.

I'm not saying we shouldn't turn off the pref in the migration code.  I'm just
saying there's a little more involved than is implied by the statment "not
migrate the pref".  

Comment 10

[selmer (gone)]

Seems like this could be a release note item.  If there's an extremely simple
fix it might have a chance.

Comment 11

[(not reading, please use [e-mail] instead)]

looking at 53080, I see that PAC does work.

I think we should migrate the proxy prefs.

53080 is [rtm need info].  if we fix that bug, this would be a WONTFIX.

Tomi, do you agree?

Comment 12

[Tomi Leppikangas]

Now if you have 4.x browser and you migrate it's prefs, mozilla
get:
user_pref("network.proxy.autoconfig_url", "http://www.oulu.fi/proxy.pac");
user_pref("network.proxy.type", 2);

When you start browser, you can't load anything, just get bunc
of js errors.

So solution for this is migrate 
user_pref("network.proxy.type", 2); 
->
user_pref("network.proxy.type", 0); 

so that autoproxy isn't used. This hack should be removed when
autoconfig is ready (bug #53080). autoconfig_url could be migrated,
it doesn't brake anything.

Comment 13

[Gagan]

I have to agree here, we should at the very least switch the auto config pref to
0 (none) That way the browser would still be usable. The release note can
describe the problem better.

Comment 14

[(not reading, please use [e-mail] instead)]

gagan knows the fallout of going to 2 -> 0 better than me.

I'll go work on that fix, and attach the patch.

Comment 15

[(not reading, please use [e-mail] instead)]

here's the patch.

Comment 16

[(not reading, please use [e-mail] instead)]

Attachment: here's the patch.

Comment 17

[(not reading, please use [e-mail] instead)]

taking from gagan.

racham / dbragg, you want to review?

note, I'm piggy backing off the UTF-8 pref conversion, since that happens only
once during migration, and never again.

Comment 18

[(not reading, please use [e-mail] instead)]

Attachment: whoops, here's the real patch.

Comment 19

[(not reading, please use [e-mail] instead)]

ignore that first patch, that patch included other fixes.

the new patch is just for this bug.

accepting.

Comment 20

[dbragg]

Patch looks good to me.  Although I don't like fixing this kind of bug in the
migration code but I guess that's what we have to do at this stage of the game.
r=dbragg

Comment 21

[(not reading, please use [e-mail] instead)]

dbragg, yeah this is a band aid fix.

if I check it in, I'll open a new bug to rip it out once we fix the real bug.

Comment 22

[(not reading, please use [e-mail] instead)]

gagan, you want to sr= this?

Comment 23

[Tomi Leppikangas]

Patch looks good, and it works as it should. Mozilla now load pages
after profile migration even if 4.x profile has autoproxy on.

Comment 24

[Scott MacGregor]

sr=mscott pending a final review from gagan as well.

Comment 25

[(not reading, please use [e-mail] instead)]

this fix would be branch only.

on the trunk, we'll let auto-config stay broken until #53080 gets fixed.

Comment 26

[Phil Peterson]

PDT marking [rtm need info]. We're unconvinced that the right approach is to
blast away the prefs, vs. changing the PAC backend to ignore the pref. Gagan or
Seth, please come see us tomorrow to talk about these two options.

Comment 27

[selmer (gone)]

Nobody responded to Phil's request.  We're fast approaching defacto rtm-...

Comment 28

[(not reading, please use [e-mail] instead)]

I'm not affected by this, and there is a workaround.

if this doesn't make rtm, it would not be the end of the world for me.

Tomi, how about for you?

gagan, this is all up to you.

my finger is on the [rtm-] button.

Comment 29

[(not reading, please use [e-mail] instead)]

marking [rtm-] to get it off the pdt radar.

gagan, if you feel otherwise talk to the pdt

Comment 30

[Tomi Leppikangas]

Our domain really needs this, many 4.x browsers use proxyautoconfig 
now, when they upgrade to 6.0, they cant visit a single page :(

This hack fixes that nicely, and users could easily put that pref back
when working browser comes. They can also use gagans workaround
for whole autoproxy thing. If that proxytype=2 is ingnored completely,
that workaround coudnt be used.

Comment 31

[Daniel Veditz [:dveditz]]

I don't think we can rtm- this, we get complaints about this all the time and 
the users are quite mystified.

Comment 32

[(not reading, please use [e-mail] instead)]

never made it for 6.0, and it's not going to land on the trunk since we should
really just fix 53080

marking wont fix.

Comment 33

[Grace Bush]

vfy wontfix