1503837 - Deleting cookie from TB cookie manager does not reflect in the panel if any cookie but the last one is deleted

Status: RESOLVED FIXED

(Thunderbird :: Preferences, defect)

Description

[Jorg K (CEST = GMT+2)]

+++ This bug was initially created as a clone of Bug #1503654 +++

Now that bug 1503654 has landed, cookies can be deleted again. However, there is no visual feedback in the panel if the first or any cookie but the last is removed. Works when removing the last cookie. It shows the correct result when opening and closing the panel again.

Is that another listbox/richlistbox issue?

Any takers?

Comment 1

[Richard Marti (:Paenglab)]

No listbox/richlistbox issue, a tree is used: https://searchfox.org/comm-central/source/mail/components/preferences/cookies.xul#42

Comment 2

[Frank-Rainer Grahl (:frg)]

I am out. Completely swamped with SeaMonkey fixes and way behind.

SeaMonkey uses a tree too and removes them in suite/components/dataman/content/dataman.js "delete: function cookies_delete() {". Maybe you can just copy some code from there.

Comment 3

[Jorg K (CEST = GMT+2)]

After landing bug 1503654 on ESR 60, it shows the same behaviour as trunk. Cookies can be deleted, but the visual feedback in the panel isn't correct. Nothing in the error console.

Comment 4

[Jorg K (CEST = GMT+2)]

Lots of ugly code in mail/components/preferences/cookies.js :: deleteCookie(). The SM code (comment #2) looks less confusing. Looking at FF, it only offers to delete *all* site date including cookies for a specific site.

So I think the time has come to remove that horrible tree that lets you delete individual cookies and offer a simple list that will remove all cookies for a site. That will solve all problems and makes more sense.

Comment 5

[Jorg K (CEST = GMT+2)]

Given that deleting cookies was broken all throughout TB 52 and no one noticed, certainly not a top priority.

Comment 6

[:aceman]

You mean last cookie from a host works fine, but if there is more of them for a host, the tree is not updated. But the cookies are removed correctly in all cases.

Comment 7

[Jorg K (CEST = GMT+2)]

Yes, although I think I saw various "tree not updated" cases. As I said, we can possibly scrap the tree and the code and remove all cookies for a site. That's what FF does. I haven't looked at FRG's suggestion in detail.

Comment 8

[:aceman]

Attachment: 1503837.patch

Took quite some time to find the trivial typo :(

We ported the originAttributes comparision function, but not the negation, see
Bug 1256153 comment 16 and https://hg.mozilla.org/mozilla-central/rev/ab3a305b3e41 (they landed something different in bug 1245184 that what is attached in the patches).

Comment 9

[Jorg K (CEST = GMT+2)]

Comment on attachment 9023638 [details] [diff] [review]
1503837.patch

Thanks, but there are two negations to remove:
https://hg.mozilla.org/comm-central/rev/89c98293e4af#l2.12
https://hg.mozilla.org/comm-central/rev/89c98293e4af#l2.31

r+ with this fixed.

Comment 10

[:aceman]

Attachment: 1503837.patch v1.1

Right, thanks.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e6e4d75191c4
Bug 1503654 follow-up: Adapt to inverted logic of ChromeUtils.isOriginAttributesEqual in cookies dialog. r=jorgk

Comment 12

[Jorg K (CEST = GMT+2)]

Pretty embarrassing we messed up in bug 1503654 :-(

Comment 13

[Jorg K (CEST = GMT+2)]

TB 60.3.1/TB 60.4 ESR
https://hg.mozilla.org/releases/comm-esr60/rev/37051da6d8fd250f2ddbe45e600d75c301436b47

Comment 14

[Jorg K (CEST = GMT+2)]

Beta (TB 64 beta 3):
https://hg.mozilla.org/releases/comm-beta/rev/5ca1b2b18ca0