1896545 - Empty Trash/Clean up ("Expunge) Inbox on Exit on an IMAP account may not work

Status: VERIFIED FIXED

(MailNews Core :: Networking: IMAP, defect, P3)

Description

[gene smith]

Attachment: authlogin-fix.diff

Re: attachment 9361598 [details] from Bug 1768344
When I shutdown daily with "empty trash on exit" or "expunge inbox on exit" often nothing happens because the referenced patch is applied to daily but was never put into release.

I think AuthLogin() needs to still allow some URLs to run (deleteallmsg, expunge inbox, etc) when shutting down rather than forbid logins for all URLs. There are other places in nsImapProtocol.cpp that allow a subset of URLs to still occur when shutdown is happening which won't prevent inbox expunge/empty trash.

I originally had the attached patch as part of bug 1862111 but decided it should be a separate change. See Bug 1768344 comment 88 for my original report of this issue.

Comment 1

[Wayne Mery (:wsmwk)]

Based on your prior comments in bug 1768344 I believe this affects 115.

Comment 2

[Wayne Mery (:wsmwk)]

Ha. I just reread the bug summary. If bug 1768344 wasn't fully implemented, how is 115 not affected?

Comment 3

[gene smith]

Re: attachment 9361598 [details] from Bug 1768344
I'm don't know how the patch affects the bug, which is something about a crash at shutdown when you have deleted all your stored passwords. So that may still occur with 115 (since one patch of the 3 was never applied) but at least 115 still allows empty trash and expunge inbox on shutdown (since one patch was never applied).
Since Magnus produced the patch that was never applied, he should probably be asked about this. I don't know if maybe not applying the one patch of the 3 was intentional or was just overlooked.

Comment 4

[Wayne Mery (:wsmwk)]

See comment 3

Comment 7

[gene smith]

Attachment: Bug 1896545 - If configured, always allow empty trash or inbox expunge to occur on shutdown. r=welpy-cw,mkmelin

Comment 9

[gene smith]

Arthur K.,
When this is in daily could you check again to make sure that you don't see crashes at shutdown with "empty trash on shutdown" enabled?
Re: Bug 1869297 comment 36 and Bug 1869297 comment 38
Thanks,
-gene

Comment 10

[Arthur K. (he/him)]

Wayne,

When is it likely to be in daily?

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/0297d0655c6f
If configured, always allow empty trash or inbox expunge to occur on shutdown. r=welpy-cw,mkmelin

Comment 12

[Wayne Mery (:wsmwk)]

(In reply to Arthur K. (he/him) from comment #10)

When is it likely to be in daily?

Tuesday's

Comment 13

[Arthur K. (he/him)]

(In reply to Wayne Mery (:wsmwk) from comment #12)

(In reply to Arthur K. (he/him) from comment #10)

When is it likely to be in daily?

Tuesday's

Wayne,

I wasn't able to test with the Daily you needed me to but I see now this is marked as fixed. Do you still want me to try on my home PC?

Comment 14

[gene smith]

Arthur K.,
It was me (Gene) asking you to test in daily and not Wayne asking.
Anyhow, this (now in daily) changes some code in 128 that may affect if there is a crash at shutdown. So if you could try it with daily and make sure that there is no crash on shutdown and the expunge of inbox occurs at shutdown it would be helpful to make sure this doesn't cause a regression. I don't see a crash at shutdown with linux but since you are on Windows (I think) and were seeing a crash is why I'm asking you to test it.
If you can't test it, probably not a big deal since without this change expunge or empty trash at shutdown often doesn't occur.
Thanks!

Comment 15

[Arthur K. (he/him)]

Gene,

When I gave the daily from ../pub/thunderbird/nightly/2024/10/2024-10-26-10-54-50-comm-central/ a shot it didn't seem to trigger the issue on my home PC. Cautiously saying that it seems to be ok. Other's mileage may vary.

Comment 16

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Confirming this issue as verified fixed with 133.0b3(20241107210012) using Windows 11, macOS 14, and Ubuntu 22.

Comment 17

[Wayne Mery (:wsmwk)]

Please nominate for uplift to 128.

Comment 18

[gene smith]

Attachment: Bug 1896545 - If configured, always allow empty trash or inbox expunge to occur on shutdown. r=welpy-cw,mkmelin

Original Revision: https://phabricator.services.mozilla.com/D224062

Comment 19

[Phabricator Automation]

esr128 Uplift Approval Request

• User impact if declined: User may be unable to auto-expunge inbox or auto-empty trash on shutdown
• Code covered by automated testing: no
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: none
• Risk associated with taking this patch: Low
• Explanation of risk level: low chance of shutdown crash
• String changes made/needed: none
• Is Android affected?: no

Comment 20

[Francesco]

Comment on attachment 9437808 [details]
Bug 1896545 - If configured, always allow empty trash or inbox expunge to occur on shutdown. r=welpy-cw,mkmelin

Correcting request. It's for comm, not mozilla. Request in comment #19.

Comment 21

[gene smith]

(In reply to Francesco from comment #20)

Comment on attachment 9437808 [details]
Bug 1896545 - If configured, always allow empty trash or inbox expunge to occur on shutdown. r=welpy-cw,mkmelin

Correcting request. It's for comm, not mozilla. Request in comment #19.

Thanks. Didn't know I had to make that selection. Thought it would know the uplift was for comm since the patch is for a "mailnews" file. I guess this explains why the raw "Lando" error shows "can't find file/file doesn't exist". Also, FYI, I don't have authorization to use "Lando" and not even sure what it is.

Comment 22

[Francesco]

Lando is the system to "land" (commit) patches. Code sheriffs use it to push patches to the code base. Of course you can't push a patch for the comm-* repo to a mozilla-* repo. The approval-mozilla-esr128? flag should be removed.

Comment 23

[Corey Bryant]

Comment on attachment 9437808 [details]
Bug 1896545 - If configured, always allow empty trash or inbox expunge to occur on shutdown. r=welpy-cw,mkmelin

[Triage Comment]
Gene, I'm going to approve the original patch since it appears that it will apply cleanly to esr128.

Comment 24

[Corey Bryant]

Comment on attachment 9427958 [details]
Bug 1896545 - If configured, always allow empty trash or inbox expunge to occur on shutdown. r=welpy-cw,mkmelin

[Triage Comment]
Approved for esr128

Comment 25

[Corey Bryant]

Thunderbird 128.5.0esr:
https://hg.mozilla.org/releases/comm-esr128/rev/ba3366063a2d