1235199 - Remove "Ask for each cookie" radiobutton in preferences dialog now that its backend is gone

Status: RESOLVED FIXED

(SeaMonkey :: Passwords & Permissions, defect)

Description

[Frank-Rainer Grahl (:frg)]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:44.0) Gecko/20100101 Firefox/44.0 SeaMonkey/2.41
Build ID: 20151224203712

Steps to reproduce:

Set the cookie Retention policy to Ask for each cookie in the Preferences dialog.


Actual results:

Instead of a dialog box asking me how I want to allow cookies for a website the first time I visit it cookies are silently stored based on the Cookie Acceptance policy. This can undermine your privacy. 


Expected results:

Bug 606655 removed the option to always ask for cookies. The behaviour should be restored for suite. Further removals are planned in Bug 1234875

2.40 branch is not affected.

Comment 1

[Frank-Rainer Grahl (:frg)]

Attachment: reverts the parts from 606655 for suite.

Patch only useful for private builds unless someone decides that reverting the original patch just in the Mozilla backend might be possible. 

Tested on Windows with en-us private builds

User agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:44.0) Gecko/20100101 Firefox/44.0 SeaMonkey/2.41
Build identifier: 20151226135002

User agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:46.0) Gecko/20100101 Firefox/46.0 SeaMonkey/2.43a1
Build identifier: 20151226133303

Comment 2

[therube]

Confirmed breakage - using the 2.41 build from ~akalla.

Comment 3

[Tony Mechelynck [:tonymec]]

These MoCo guys have really decided that SeaMonkey doesn't exist. Not only the official company policy is that it's forbidden to help SM & TB, now they're positively throwing wrenches into our cogs. :-/

See also 787208 and tb-planning thread https://groups.google.com/d/topic/tb-planning/YTsw5LRfMZc/discussion

This patch is for mozilla-central, not comm-central, is that it?

IMHO getting back the "ask me" option for cookies in the official Suite builds would be desirable. Whether it would be possible is a different question. Maybe only after bug c-c-m-c-merge gets FIXED, by reverting the changeset ba589252e3f4 (or equivalent) as well as whichever changeset "fixes" bug 1234875 in our embedded mozilla-central clone, which at that time won't need anymore to be identical with Firefox's. This might hang on a policy decision on SeaMonkey's part. Callek? ewong? Neil?

Comment 4

[Tony Mechelynck [:tonymec]]

s/787208/bug 787208/

Comment 5

[Frank-Rainer Grahl (:frg)]

>> This patch is for mozilla-central, not comm-central, is that it?

Yes so it's no good here. Just for reference what's missing. If someone thinks that it's worth to file a bug for the backend it could be used to restore the needed functionality. Applies clean to current m-c, m-a and with minor adjustments also to m-b. I didn't even try. 2.42 is also affected.

Comment 6

[Tony Mechelynck [:tonymec]]

(In reply to Frank-Rainer Grahl from comment #5)
> >> This patch is for mozilla-central, not comm-central, is that it?
> 
> Yes so it's no good here. Just for reference what's missing. If someone
> thinks that it's worth to file a bug for the backend it could be used to
> restore the needed functionality. Applies clean to current m-c, m-a and with
> minor adjustments also to m-b. I didn't even try. 2.42 is also affected.

IIUC at this time a Core bug filed to revert preferences backend functionalities removed two months ago because Firefox doesn't use them would get WONTFIXed at whizzbang speed. (I hope I'm wrong but I'm entertaining no illusions about this.) OTOH if we can eventually both fork mozilla-central and keep importing any _useful_ fixes in it…

Comment 7

[Philip Chee]

(In reply to Boris Zbarsky [:bz] (Vacation until Jan 4) from comment #4)
> Note that any users who set the pref are probably crashing multiple times a
> day as a result.... so I expect the number of such users who are still using
> the browser is pretty low.
Are we seeing any crashes due to this?

Bug 570366 - can not open a link if set cookies "ask me every time"
WONTFIXED. Or rather fixed by Bug 606655
Does this affect SeaMonkey? If so can this be fixed?

I will start a new thread cross-posted to mozilla.dev.apps.seamonkey && mozilla.dev.apps.thunderbird about the issues raised here.

Comment 8

[Edmund Wong (:ewong)]

(In reply to Frank-Rainer Grahl from comment #1)
> Created attachment 8702067 [details] [diff] [review]
> reverts the parts from 606655 for suite.
> 
> Patch only useful for private builds unless someone decides that reverting
> the original patch just in the Mozilla backend might be possible. 
> 
> Tested on Windows with en-us private builds
> 
> User agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:44.0) Gecko/20100101
> Firefox/44.0 SeaMonkey/2.41
> Build identifier: 20151226135002
> 
> User agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:46.0) Gecko/20100101
> Firefox/46.0 SeaMonkey/2.43a1
> Build identifier: 20151226133303

This will need to be reviewed by a DOM peer but since the original patch
from bug 606655 was fixed, I'm not sure this will be favorably reviewed.

I'm wondering if it's possible to take the code that this bug depends on and
place it within suite/?  (in essence, a mini fork)  (As it is part of the main
DOM code, I'm not sure if it's possible; just a suggestion since we'd need to
convince the DOM peers/owner that we need this functionality.

Comment 9

[Philip Chee]

For releases we can create a SeaMonkey specific release branch in mozilla-release and apply our patches there. But someone will have to fix any merge conflicts. And *remember* to apply said patches after every uplift. And test that we didn't regress anything (that we don't want regressed).

Comment 10

[Frank-Rainer Grahl (:frg)]

Maybe someone with some standing should open a core bug with this patch and see what happens. Maintaining patches is not fun. If the bug is shot down planning for a future without Mozilla should maybe accelerated. 

I can not reproduce Bug 570366 with Seamonkey. The next time a cookie is needed a dialog will just be shown. I use the setting now for more than 10 years and the only crashed I experienced lately are in the Download Manager see Bug 1224326 . For Firefox it can still be left closed because the ui doens't expose it.

FRG

Comment 11

[Justin Wood (:Callek)]

I'm not a fan of repeated forking, just for maintenance's sake. It gets hard. Maintaining relbranches and overall state on them also gets hard. and easily overlooked pieces.

That said, I don't feel I have much stake in this particular pie (I've never much liked the 'ask me first' aspect since its like saying 'you need to pay to go in there, but we won't tell you whats in there until you go in, and yes you have to pay to go in, no refunds')

Comment 13

[[e-mail]]

I don't think we can seriously contemplate forking core functionality like this, so all we can do is to remove the now useless UI. Sorry about that.

Comment 14

[Tony Mechelynck [:tonymec]]

(In reply to [e-mail] from comment #13)
> I don't think we can seriously contemplate forking core functionality like
> this, so all we can do is to remove the now useless UI. Sorry about that.

OK, so what remains to be done is to remove one radio-button item and its functionality in the prefpane with id="cookies_pane" at chrome://communicator/content/pref/preferences.xul — I'm willing to try. MXR tells me it's at line 78 sqq of comm-central/source/suite/common/pref/pref-cookies.xul

Comment 15

[Frank-Rainer Grahl (:frg)]

How about filing a dom bug first? If it's shot down the functionality can still be removed later.

FRG

Comment 16

[Tony Mechelynck [:tonymec]]

(In reply to Frank-Rainer Grahl from comment #15)
> How about filing a dom bug first? If it's shot down the functionality can
> still be removed later.
> 
> FRG

The functionality in Core has already been removed. What's need to be done is to clean up the SeaMonkey pref panel.

Comment 17

[Tony Mechelynck [:tonymec]]

well, Core or Networking or Toolkit… something common to Fx & Sm anyway.

Comment 18

[Tony Mechelynck [:tonymec]]

Attachment: remove radiobutton, checkbox, and their strings

Comment 19

[Tony Mechelynck [:tonymec]]

The default is not set to the removed value so that's all right. I suppose we can't do anything about users who had set the pref to "Ask me" and now find themselves accepting all cookies: AFAIK that's in mozilla-central, out of our ballpark.

Comment 20

[[e-mail]]

Comment on attachment 8703304 [details] [diff] [review]
remove radiobutton, checkbox, and their strings

Need to remove the <preference> elements too.

Comment 21

[Tony Mechelynck [:tonymec]]

(In reply to [e-mail] from comment #20)
> Comment on attachment 8703304 [details] [diff] [review]
> remove radiobutton, checkbox, and their strings
> 
> Need to remove the <preference> elements too.

Oops, my bad. The lifetimePolicy pref is still needed for the radiobuttons not removed, but the lifetimeDays can indeed go away. New patch coming.

Comment 22

[Tony Mechelynck [:tonymec]]

Attachment: remove radiobutton, checkbox and strings + unneeded pref reference

Comment 23

[Tony Mechelynck [:tonymec]]

Comment on attachment 8703385 [details] [diff] [review]
remove radiobutton, checkbox and strings + unneeded pref reference

Sh.. Removed wrong pref.

Comment 24

[Tony Mechelynck [:tonymec]]

Attachment: patch v3

let's hope I didn't goof this time

Comment 25

[[e-mail]]

(In reply to comment #20)
> Need to remove the <preference> elements too.

Oh, of course, only one of those controls has a preference element, so I mistakenly pluralised the word element. Sorry about that.

Comment 26

[Tony Mechelynck [:tonymec]]

(In reply to [e-mail] from comment #25)
> (In reply to comment #20)
> > Need to remove the <preference> elements too.
> 
> Oh, of course, only one of those controls has a preference element, so I
> mistakenly pluralised the word element. Sorry about that.

There are two preference elements involved, but one of them is still required for the remaining radiobuttons. I see that we agree. :-)

Comment 27

[Tony Mechelynck [:tonymec]]

Do we really have to wait for bug 1234875? The rationale for that bug is that "mCookiesAlwaysAcceptSession is assigned to but never used". This bug would remove one of the cases where it is assigned to, IIUC we can do it at any time. The dependency might even be reversed; but I'm not going to spam the Core devs by blocking them from a SeaMonkey bug.

Also, "blocker" looks widely overstated to me. If I had to evaluate it I'd say "trivial" (cosmetic problem in the security prefs dialog). I guess I should have downgraded this Severity at comment #14, after it was decided at comment #13 that there was nothing we could do to get back the pref removed in Core. Sorry, Frank: once the backend is gone, it's gone; no use crying over spilt milk.

Comment 28

[[e-mail]]

Comment on attachment 8703386 [details] [diff] [review]
patch v3

Sorry, I meant to review this after making my previous comment, but I got distracted by other stuff.

Comment 29

[Tony Mechelynck [:tonymec]]

Attachment: patch for aurora & beta (with orphaned string entities left in)

This "aurora/beta patch" is the first part of the comm-central patch, i.e. the removal of the XUL elements in the prefs dialog.

The entity definitions for the strings are intentionally left in, in the hope not to disturb any translation automata.

This patch was made against the current comm-aurora default head (as of this writing). It is expected to apply cleanly on beta too: IIUC this is code dating from the Toolkit Transition.

Approval Request Comment
[Feature/regressing bug #]: Fx bug 606655, Sm bug 1235199
[User impact if declined]: useless UI in prefs dialog (radiobutton & checkbox whose backends were removed in Gecko 44)
[Describe test coverage new/current, TreeHerder]: n/a
[Risks and why]: none (removal of UI which already did nothing)
[String/UUID change made/needed]: a few strings become unused, but this patch leaves their entities defined, hoping not to disturb the i18n automata.

Comment 30

[[e-mail]]

Comment on attachment 8705943 [details] [diff] [review]
patch for aurora & beta (with orphaned string entities left in)

rs=me

Comment 31

[Tony Mechelynck [:tonymec]]

Could someone please check this in? Tomorrow one week will have passed since comment #28.

Comment 32

[Philip Chee]

http://hg.mozilla.org/comm-central/rev/df44c29bbbe0
http://hg.mozilla.org/releases/comm-aurora/rev/1cd3aa3d4744
http://hg.mozilla.org/releases/comm-beta/rev/1f666e9604ba

Comment 33

[Mart Rootamm]

Has there been a bug filed to return the backend in Firefox? Because Bug 606655 made it to Slashdot.

If cookie handling with "Ask me every time" is to be returned to SeaMonkey, then I could make some UI suggestions, and move to using SeaMonkey.

Comment 34

[Frank-Rainer Grahl (:frg)]

>> Has there been a bug filed to return the backend in Firefox? 

Not that I am aware of it. But just go ahead and file one. 

With the old Seamonkey Cookie Manager you can cureently at least get rid of them easier than in FF:

chrome://communicator/content/permissions/cookieViewer.xul

It complements the standard Data manager quite nicely.

FRG

Comment 35

[Tony Mechelynck [:tonymec]]

(In reply to Frank-Rainer Grahl from comment #34)
> (In reply to Mart Rootamm from comment #33)
> > Has there been a bug filed to return the backend in Firefox? 
> 
> Not that I am aware of it. But just go ahead and file one.

Be aware though, that pleading for reversal of a recently FIXED Firefox or Core bug runs a high risk of being summarily RESOLVED INVALID by some triager, except maybe (only maybe) if filed with extremely convincing argumentation by some highly respected developer. Neil might swing it but I don't think he'll take the trouble to. I wouldn't even try. You… well, as Frank-Rainer said, go ahead and see what happens; and if you do, please make your new bug "block" this one (and bug 1245110) so anyone interested with the present bug can see what happens about rolling it back.

Comment 36

[Mart Rootamm]

(In reply to Tony Mechelynck [:tonymec] from comment #35)

Right, someone got there before me, and submitted Bug 1249151. That was quickly given a RESOLVED WONTFIX status, and then changed back to UNCONFIRMED.

Comment 37

[kokoro]

As Yang putted it to RESOLVED WONTFIX, i selected the only choice i had "UNCONFIRMED" to put it back on the track, if the procedure may allow it! :) Thank you for the follow up ^^

Comment 38

[Tony Mechelynck [:tonymec]]

Bug 1249151 then got again RESOLVED WONTFIX and even VERIFIED WONTFIX twice by Marco Bonardo, who happens to be a Firefox peer and thus /ex officio/ a Toolkit peer (see https://wiki.mozilla.org/Modules/All), so if at the time of comment #35 your chances were IMHO low, now it can be said for certain that they are zero, zilch, nought, nil, nonexistent.

Any attempt by a single user to repeatedly REOPEN such a bug recently VERIFIED WONTFIX would be regarded as obstination against the bugzilla.mozilla.org rules and could, if repeated against repeated warnings, incur revocation of the BMO account. So don't try, you have better things to do.

In theory, it might be possible to add back somewhere in comm-central the code that was removed somewhere in mozilla-central, but after comment #13 above it won't happen. You liked this feature, and I liked too, but we have to do the best we can with what we've got.

Comment 39

[Frank-Rainer Grahl (:frg)]

Well I now see how Australis came to this world and why the FF market share goes down steadily.

I will maintain it as a private patch for my builds as long as I can which till now is a joke for such an 'unmaintained' fature. Still works well in 2.44a1. If someone needs an updated source patch agains m-c and c-c just let me know.

FRG