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

RESOLVED FIXED in seamonkey2.43

Status

defect
--
trivial
RESOLVED FIXED
4 years ago
2 months ago

People

(Reporter: frg, Assigned: tonymec)

Tracking

SeaMonkey 2.41 Branch
seamonkey2.43
Dependency tree / graph

SeaMonkey Tracking Flags

(seamonkey2.40 unaffected, seamonkey2.41 fixed, seamonkey2.42 fixed, seamonkey2.43 fixed)

Details

Attachments

(2 attachments, 3 obsolete attachments)

Reporter

Description

4 years ago
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.
Reporter

Updated

4 years ago
Depends on: 606655
Reporter

Updated

4 years ago
Severity: normal → blocker
Depends on: 1234875
OS: Unspecified → All
Priority: -- → P1
Hardware: Unspecified → All
Reporter

Comment 1

4 years ago
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

4 years ago
Confirmed breakage - using the 2.41 build from ~akalla.
Status: UNCONFIRMED → NEW
Ever confirmed: true
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?
Flags: needinfo?(neil)
Flags: needinfo?(ewong)
Flags: needinfo?(bugspam.Callek)
Version: SeaMonkey 2.41 Branch → Trunk
Reporter

Comment 5

4 years ago
>> 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.
(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

4 years ago
(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.
(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.
Flags: needinfo?(ewong)

Comment 9

4 years ago
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).
Reporter

Comment 10

4 years ago
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
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')
Flags: needinfo?(bugspam.Callek)

Updated

4 years ago
Summary: Cookie preferences broken in Seamonkey → Cookie preferences broken in Seamonkey ("Ask for each cookie" does not work)

Updated

4 years ago
Duplicate of this bug: 1231105

Updated

4 years ago
Version: Trunk → SeaMonkey 2.41 Branch
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.
Flags: needinfo?(neil)
(In reply to neil@parkwaycc.co.uk 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
Assignee: nobody → antoine.mechelynck
Status: NEW → ASSIGNED
Component: General → Passwords & Permissions
Summary: Cookie preferences broken in Seamonkey ("Ask for each cookie" does not work) → Remove "Ask for each cookie" radiobutton in preferences dialog now that its backend is gone
Reporter

Comment 15

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

FRG
(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.
well, Core or Networking or Toolkit… something common to Fx & Sm anyway.
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 on attachment 8703304 [details] [diff] [review]
remove radiobutton, checkbox, and their strings

Need to remove the <preference> elements too.
Attachment #8703304 - Flags: review?(neil) → review-
(In reply to neil@parkwaycc.co.uk 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 on attachment 8703385 [details] [diff] [review]
remove radiobutton, checkbox and strings + unneeded pref reference

Sh.. Removed wrong pref.
Attachment #8703385 - Attachment is obsolete: true
Attachment #8703385 - Flags: review?(neil)
Posted patch patch v3Splinter Review
let's hope I didn't goof this time
Attachment #8703386 - Flags: review?(neil)
(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.
(In reply to neil@parkwaycc.co.uk 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. :-)
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.
Severity: blocker → trivial
No longer depends on: 1234875
Priority: P1 → --
See Also: → 1234875
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.
Attachment #8703386 - Flags: review?(neil) → review+
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.
Attachment #8705943 - Flags: review?(neil)
Attachment #8705943 - Flags: approval-mozilla-beta?
Attachment #8705943 - Flags: approval-mozilla-aurora?
Attachment #8705943 - Flags: approval-mozilla-beta?
Attachment #8705943 - Flags: approval-mozilla-aurora?
Attachment #8705943 - Flags: approval-comm-beta?
Attachment #8705943 - Flags: approval-comm-aurora?
Comment on attachment 8705943 [details] [diff] [review]
patch for aurora & beta (with orphaned string entities left in)

rs=me
Attachment #8705943 - Flags: review?(neil) → review+
Could someone please check this in? Tomorrow one week will have passed since comment #28.
Flags: needinfo?(iann_bugzilla)

Updated

4 years ago
Attachment #8705943 - Flags: approval-comm-beta?
Attachment #8705943 - Flags: approval-comm-beta+
Attachment #8705943 - Flags: approval-comm-aurora?
Attachment #8705943 - Flags: approval-comm-aurora+
Flags: needinfo?(iann_bugzilla)

Updated

3 years ago
Blocks: 1245110

Comment 33

3 years ago
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.
Reporter

Comment 34

3 years ago
>> 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
(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

3 years ago
(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

3 years ago
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 ^^

Updated

3 years ago
Depends on: 1249151
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.
Reporter

Comment 39

3 years ago
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
Reporter

Updated

2 months ago
Regressions: 1546747
You need to log in before you can comment on or make changes to this bug.