221078 - unfork firebird/mozilla cookie prefs

Status: RESOLVED FIXED

(Core :: Networking: Cookies, defect)

Description

[[e-mail]]

this is a spinoff of bug 170705 (i'm cc'ing the folks that were listed on that bug).

we really need to fix the forkage between firebird and mozilla in cookies and
nsGlobalWindow.cpp (see
http://lxr.mozilla.org/seamonkey/source/dom/src/base/nsGlobalWindow.cpp#6062).
having #ifdef MOZ_PHOENIX in these places is completely bogus, and prevents them
 from becoming part of the GRE. (cookies will be moving into necko, and thus the
GRE, very soon).

there's absolutely no need for these prefs to be forked, apart from the fact
that it correlates well with the respective UI's of the two products. so the
simplest thing to do is just unfork them.

there are three prefs involved: firebird has network.cookie.enable and
network.cookie.enableForOriginatingWebsiteOnly, both booleans; whereas mozilla
has network.cookie.cookieBehavior, an integer pref which is a superset of the
two firebird prefs. (firebird has two boolean checkboxes, whereas mozilla has a
set of radiobuttons).

unforking the prefs will break one of the products (those cookie prefs will be
lost, and the user will have to set them again). it's easier to break firebird.
depending on how the firebird folk feel about this, we can introduce migration
code to convert the prefs. in addition, we will need to introduce a tad of
prefpanel code to munge the two UI checkbox boolean values into one integer value.

comments on this proposal would be much appreciated.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Ben should probably be in on this discussion, as should some SeaMonkey ui people...

Comment 2

[[e-mail]]

Attachment: unfork backend prefs (landed)

this unforks the firebird "network.cookie.enabled" and
"network.cookie.enabledForOriginatingWebsiteOnly" prefs, and replaces them with
the seamonkey "network.cookie.cookieBehavior" integer pref. together with some
patches in other bugs, this will solve all cookie-related forkage for necko and
the GRE.

Comment 3

[Darin Fisher]

so is there a bug on fixing firebird's UI?

Comment 4

[[e-mail]]

Attachment: suggested fix for firebird prefpanel

this fixes the firebird prefpanel bustage resulting from unforking the prefs.
i'm not going to push this patch, because i don't have the authority to... it's
really up to the firebird people as to how they want to resolve it. it's just
what i think is the best solution. i've tested it, and it appears to work fine.


the patch preserves firebird's checkbox layout for the "enable cookies" and
"enable cookies for originating website" options, and then munges those two
booleans into an integer for the "network.cookie.cookieBehavior" pref. from
what has been explained to me, the easiest way to do this is to create a
"display:none" set of radioboxes that actually interface to that pref, and then
have the checkboxes update those radioboxes when they change. this leverages
prefpanel code to preserve those values when another panel is selected. (just
having the checkboxes call a function to update the pref, and not having the
non-displayed radioboxes, won't work). there is also a few lines of code to
delete the old firebird prefs (this is optional).

of course, the user will still lose their old cookie prefs with this patch, but
it preserves UI behavior and keeps the pref database tidy.

Comment 5

[[e-mail]]

>so is there a bug on fixing firebird's UI?

sure, why not do that here?

Comment 6

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 132903 [details] [diff] [review]
unfork backend prefs (landed)

r=caillon, but per our discussion move the define in globalwindow outside of
the method somewhere?  And tack on braces on the if to follow the preferred
style in the module.

Comment 7

[Pierre Chanial]

> i'm not going to push this patch
I will! :o) Thanks dan for the patch!
dwitte, could you provide a new patch to address caillon's review?
darin, could you please sr?
:op

Comment 8

[[e-mail]]

no need to post a new patch just for those nitfixes... so the sr request still
stands.

ben goodger said he was ok with the firebird changes (but don't hold me to
that). thanks for the help pch :)

Comment 9

[Christopher Aillon (sabbatical, not receiving bugmail)]

Pch, I think the firebird patch could probably land before the backend patch
without breaking things.  Which might be prudent if you can arrange it.

Comment 10

[Darin Fisher]

Comment on attachment 132903 [details] [diff] [review]
unfork backend prefs (landed)

yeah baby!  sr=darin

Comment 11

[[e-mail]]

Comment on attachment 132903 [details] [diff] [review]
unfork backend prefs (landed)

this has landed (with nitfixes). ben/pch, i suggest you check in the firebird
patch sometime soon ;)

Comment 12

[Pierre Chanial]

Great!
I'll take care of the Firebird part.
A big thank you to Dan and the reviewers for removing this BIG nonsense!

Comment 13

[[e-mail]]

no problem. donations of beer accepted.

Comment 14

[[e-mail]]

*** Bug 222599 has been marked as a duplicate of this bug. ***

Comment 15

[Pierre Chanial]

I checked in the Firebird part after having to reinstall my distro, build
environment etc...
I used a <data/> element instead of a <radiogroup/> to store the pref value. I
also refactored the code to clearly distinguish the update of the broadcaster
and the pref value (then avoiding an unnecessary call). Some renaming and other
nits. This patch also migrate the pref so that new users won't lose their
preferences.

Comment 16

[Walter K.]

Should Firebird's other two boolean cookie prefs be folded into the same new int
pref? Such change seems a logical extension to the current change.

Comment 17

[[e-mail]]

nice work pch! i learnt some new xul today. ;)

walter: no. there's no point in changing the other prefs. they're fine the way
they are

Comment 18

[Pierre Chanial]

dwitte: you're welcome! Beers are waiting in the fridge. But hurry up, they
won't last... :)