Closed
Bug 1235199
Opened 9 years ago
Closed 9 years ago
Remove "Ask for each cookie" radiobutton in preferences dialog now that its backend is gone
Categories
(SeaMonkey :: Passwords & Permissions, defect)
Tracking
(seamonkey2.40 unaffected, seamonkey2.41 fixed, seamonkey2.42 fixed, seamonkey2.43 fixed)
RESOLVED
FIXED
seamonkey2.43
Tracking | Status | |
---|---|---|
seamonkey2.40 | --- | unaffected |
seamonkey2.41 | --- | fixed |
seamonkey2.42 | --- | fixed |
seamonkey2.43 | --- | fixed |
People
(Reporter: frg, Assigned: tonymec)
References
Details
Attachments
(2 files, 3 obsolete files)
(deleted),
patch
|
neil
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
neil
:
review+
philip.chee
:
approval-comm-aurora+
philip.chee
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
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•9 years ago
|
Severity: normal → blocker
Depends on: 1234875
OS: Unspecified → All
Priority: -- → P1
Hardware: Unspecified → All
Reporter | ||
Comment 1•9 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
Confirmed breakage - using the 2.41 build from ~akalla.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 3•9 years ago
|
||
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?
Assignee | ||
Updated•9 years ago
|
status-seamonkey2.40:
--- → unaffected
status-seamonkey2.41:
--- → affected
status-seamonkey2.43:
--- → affected
Flags: needinfo?(neil)
Flags: needinfo?(ewong)
Flags: needinfo?(bugspam.Callek)
Version: SeaMonkey 2.41 Branch → Trunk
Assignee | ||
Comment 4•9 years ago
|
||
s/787208/bug 787208/
Reporter | ||
Comment 5•9 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.
Assignee | ||
Comment 6•9 years ago
|
||
(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…
status-seamonkey2.42:
--- → affected
Comment 7•9 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.
Comment 8•9 years ago
|
||
(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•9 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•9 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
Comment 11•9 years ago
|
||
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•9 years ago
|
Summary: Cookie preferences broken in Seamonkey → Cookie preferences broken in Seamonkey ("Ask for each cookie" does not work)
Updated•9 years ago
|
Version: Trunk → SeaMonkey 2.41 Branch
Comment 13•9 years ago
|
||
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)
Assignee | ||
Comment 14•9 years ago
|
||
(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•9 years ago
|
||
How about filing a dom bug first? If it's shot down the functionality can still be removed later.
FRG
Assignee | ||
Comment 16•9 years ago
|
||
(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.
Assignee | ||
Comment 17•9 years ago
|
||
well, Core or Networking or Toolkit… something common to Fx & Sm anyway.
Assignee | ||
Comment 18•9 years ago
|
||
Attachment #8702067 -
Attachment is obsolete: true
Attachment #8703304 -
Flags: review?(neil)
Assignee | ||
Comment 19•9 years ago
|
||
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•9 years ago
|
||
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-
Assignee | ||
Comment 21•9 years ago
|
||
(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.
Assignee | ||
Comment 22•9 years ago
|
||
Attachment #8703304 -
Attachment is obsolete: true
Attachment #8703385 -
Flags: review?(neil)
Assignee | ||
Comment 23•9 years ago
|
||
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)
Assignee | ||
Comment 24•9 years ago
|
||
let's hope I didn't goof this time
Attachment #8703386 -
Flags: review?(neil)
Comment 25•9 years ago
|
||
(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.
Assignee | ||
Comment 26•9 years ago
|
||
(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. :-)
Assignee | ||
Comment 27•9 years ago
|
||
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•9 years ago
|
||
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+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 29•9 years ago
|
||
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?
Assignee | ||
Updated•9 years ago
|
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 30•9 years ago
|
||
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+
Assignee | ||
Comment 31•9 years ago
|
||
Could someone please check this in? Tomorrow one week will have passed since comment #28.
Flags: needinfo?(iann_bugzilla)
Updated•9 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+
Comment 32•9 years ago
|
||
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
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.43
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(iann_bugzilla)
Comment 33•9 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•9 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
Assignee | ||
Comment 35•9 years ago
|
||
(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•9 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•9 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 ^^
Assignee | ||
Comment 38•9 years ago
|
||
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•9 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
You need to log in
before you can comment on or make changes to this bug.
Description
•