[News] "Messages more than x days old" (ageLimit) always reverts to value 1

VERIFIED FIXED in Thunderbird 28.0

Status

MailNews Core
Account Manager
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: InvisibleSmiley, Assigned: aceman)

Tracking

Trunk
Thunderbird 28.0

Thunderbird Tracking Flags

(thunderbird27 fixed, thunderbird_esr2427+ fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

4.25 KB, patch
neil@parkwaycc.co.uk
: review+
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
Using the latest trunk nightly of either SM or TB, the following can be observed:

Go to Mail & Newsgroups Account Settings -> (some news account, e.g. news.mozilla.org) -> Synchronization & Storage -> Disk Space. There are two integer input fields in the upper half of the dialog:

1. Messages larger than [___] KB

* textbox ID: offline.notDownloadMin
* pref: mail.server.serverX.max_size
* internally stored in: gIncomingServer.maxMessageSize
* read through function: initServerSettings()
* default: http://mxr.mozilla.org/comm-central/source/mailnews/mailnews.js#395

The value of this one is saved and restored correctly. I only included it for comparison.

2. Messages more than [___] days old

* textbox ID: nntp.downloadMsgMin
* pref: mail.server.serverX.ageLimit
* internally stored in: downloadSettings.ageLimitOfMsgsToDownload
* read through functions: initDownloadSettings(),
                          nsMsgIncomingServer::GetDownloadSettings()
* default: none

The value of this one is saved correctly but always displayed as 1 after close + reopen of Account Settings. Hence, if the dialog is left with OK again in such a state, mail.server.serverX.ageLimit will be set to 1 even if it had a different value before and you did not actively change it.

I suspect the issue is somewhere in the back-end since initDownloadSettings() would otherwise set the displayed value to 30. Unfortunately I don't feel too at home in C++ land. Please help.

I haven't yet checked whether this is a regression, but I wouldn't be surprised.
(Assignee)

Comment 1

5 years ago
Confirming the bug and I'll check this out.
Assignee: nobody → acelists
(Assignee)

Comment 2

5 years ago
Created attachment 8338857 [details] [diff] [review]
patch

Seems to be a mismatch between numeric value of the object member and setting it as string into the "value" attribute. I changed it to use the .value property. The I found similar change was done for other field on the pane in bug 485785.
I also fix another textbox "remove bodies of messages older than X days old" that has the same bug. The other lines changed are reindenting if you don't mind.
Attachment #8338857 - Flags: review?(neil)
Attachment #8338857 - Flags: review?(iann_bugzilla)
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED

Comment 3

5 years ago
Comment on attachment 8338857 [details] [diff] [review]
patch

Please use .value for consistency with retention.js (speaking of which, spot the bug in retention.js that's been there since Thunderbird 1.1); valueNumber doesn't really gain us anything here.
Attachment #8338857 - Flags: review?(neil) → review+
(Assignee)

Comment 4

5 years ago
Created attachment 8340095 [details] [diff] [review]
patch v2

This bug?
Attachment #8338857 - Attachment is obsolete: true
Attachment #8338857 - Flags: review?(iann_bugzilla)
Attachment #8340095 - Flags: review?(neil)

Updated

5 years ago
Attachment #8340095 - Flags: review?(neil) → review+
(Assignee)

Comment 5

5 years ago
Thanks.
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/91bfcd1f97cd
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 28.0
(Reporter)

Comment 7

5 years ago
Verified with 20131204003001 SM trunk nightly.
Status: RESOLVED → VERIFIED
(Assignee)

Comment 8

5 years ago
Comment on attachment 8340095 [details] [diff] [review]
patch v2

[Approval Request Comment]
Regression caused by (bug #): unknown, looks like this code was there forever.
User impact if declined: if the user does not notice the silent change of the nntp.removeBodyMin value to 1 day, TB will remove messages older than 1 day, which may be dataloss to the user.
Testing completed (on c-c, etc.): SM and TB trunk
Risk to taking this patch (and alternatives if risky): small targeted fix that should be low risk and good polishing material for stable branch
Attachment #8340095 - Flags: approval-comm-esr24?
Attachment #8340095 - Flags: approval-comm-aurora?
Comment on attachment 8340095 [details] [diff] [review]
patch v2

a=Standard8. We'll get this into 24 at the next cycle.
Attachment #8340095 - Flags: approval-comm-aurora? → approval-comm-aurora+
Comment on attachment 8340095 [details] [diff] [review]
patch v2

[Triage Comment]
Looks like this didn't make it onto aurora before the merge, so updating approval to beta.
Attachment #8340095 - Flags: approval-comm-aurora+ → approval-comm-beta+
https://hg.mozilla.org/releases/comm-beta/rev/3ae1c0abfc18
status-thunderbird27: --- → fixed
tracking-thunderbird_esr24: --- → 27+
Attachment #8340095 - Flags: approval-comm-esr24? → approval-comm-esr24+
You need to log in before you can comment on or make changes to this bug.