Closed Bug 1503837 Opened Last year Closed Last year

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

Categories

(Thunderbird :: Preferences, defect)

50 Branch
defect
Not set

Tracking

(thunderbird_esr6064+ fixed, thunderbird64 fixed, thunderbird65 fixed)

RESOLVED FIXED
Thunderbird 65.0
Tracking Status
thunderbird_esr60 64+ fixed
thunderbird64 --- fixed
thunderbird65 --- fixed

People

(Reporter: jorgk, Assigned: aceman)

References

Details

Attachments

(1 file, 1 obsolete file)

+++ 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?
Flags: needinfo?(richard.marti)
Flags: needinfo?(frgrahl)
Flags: needinfo?(acelists)
No listbox/richlistbox issue, a tree is used: https://searchfox.org/comm-central/source/mail/components/preferences/cookies.xul#42
Flags: needinfo?(richard.marti)
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.
Flags: needinfo?(frgrahl)
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.
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.
Given that deleting cookies was broken all throughout TB 52 and no one noticed, certainly not a top priority.
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.
Flags: needinfo?(acelists)
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.
Attached patch 1503837.patch (obsolete) — Splinter Review
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).
Assignee: nobody → acelists
Status: NEW → ASSIGNED
Attachment #9023638 - Flags: review?(jorgk)
Blocks: 1503654
No longer depends on: 1503654
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.
Attachment #9023638 - Flags: review?(jorgk) → review+
Right, thanks.
Attachment #9023638 - Attachment is obsolete: true
Attachment #9023741 - Flags: review+
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/e6e4d75191c4
Bug 1503654 follow-up: Adapt to inverted logic of ChromeUtils.isOriginAttributesEqual in cookies dialog. r=jorgk
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Pretty embarrassing we messed up in bug 1503654 :-(
Attachment #9023741 - Flags: approval-comm-esr60+
Attachment #9023741 - Flags: approval-comm-beta+
Target Milestone: --- → Thunderbird 65.0
You need to log in before you can comment on or make changes to this bug.