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

Status: VERIFIED FIXED

(MailNews Core :: Account Manager, defect)

Description

[Jens Hatlak (:InvisibleSmiley)]

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.

Comment 1

[:aceman]

Confirming the bug and I'll check this out.

Comment 2

[:aceman]

Attachment: 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.

Comment 3

[[e-mail]]

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.

Comment 4

[:aceman]

Attachment: patch v2

This bug?

Comment 5

[:aceman]

Thanks.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/91bfcd1f97cd

Comment 7

[Jens Hatlak (:InvisibleSmiley)]

Verified with 20131204003001 SM trunk nightly.

Comment 8

[:aceman]

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

Comment 9

[Mark Banner (:standard8)]

Comment on attachment 8340095 [details] [diff] [review]
patch v2

a=Standard8. We'll get this into 24 at the next cycle.

Comment 10

[Mark Banner (:standard8)]

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.

Comment 11

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-beta/rev/3ae1c0abfc18

Comment 12

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-esr24/rev/b35998bf7f4b