1028535 - Trash folder picker not disabled in account manager -> imap account -> server settings when "Just mark it as deleted" is selected

Status: NEW

(MailNews Core :: Account Manager, defect)

Description

[:aceman]

When opening account manager -> imap account -> "server settings" and the option "Just mark it as delete" radio option is initially set, the folder picker for the "Move it to this folder" is enabled. This only happens when the option is read from the prefs when first opening the dialog. When clicking the options manually after that, the picker gets disabled right.

This seems to be an interaction of bug 878805 (folderWidgets.xml::selectFolder() removes the "disabled" attribute from the picker) and the code in setupImapDeleteUI() that tries to select a non-existent folder (there is a comment about it).

It is up for debate whether the picker should remove the "disabled" from itself when it was set onto it from outside code. In the account manager it is common practice to disable the picker when it should not be operated by the user so it is not expected that it re-enables itself randomly. It is expected that non-existent folders are selected in disabled pickers as it is quite possible the user didn't yet initialize that picker/option to a valid folder.

Comment 1

[Wayne Mery (:wsmwk)]

Should this block TB31?

Comment 2

[:aceman]

Sorry for botched summary again.

Comment 3

[:aceman]

Attachment: 1028535.patch

Hi, alta, you added this line in bug 878805, do you remember the use case that it wanted to cover?

Comment 4

[alta88]

Comment on attachment 9036061 [details] [diff] [review]
1028535.patch

if a dialog with a folderpicker was showing, and there were no subfolders, the parent menupopup would be disabled. if a subfolder were then created i would think the new subfolder should be discovered and so the parent menupopup should no longer be disabled. i don't know the conditions under which these menus are torn down anymore, perhaps a folder added notification causes a teardown/rebuild of the whole thing or just the parent menu. if you try that and it works without that line, then it can go, otherwise no.

i don't know the status of rdf anymore, but it used to be that the cache falsely returned a folder that was deleted from the filesystem (without a restart). so picking it would fail when it was accessed on disk later. meaning a folder should really be tested with exists() or something for this edge case when actually picking a folder (if not when populating the menus).

Comment 5

[:aceman]

OK, that seems useful to enable the picker if a folder becomes available.
So we want the picker to manage itself, but not if the parent disabled it intentionally. Then it should not enable itself.

So either we invent a new attribute (not named "disabled") that the caller sets, or we add an internal property that tracks whether the picker disabled itself or whether it was done externally.

Comment 6

[alta88]

or have a container that could be disabled (separately and by caller) which state was inherited by the child menu. at least you can do this with html <fieldset> ;)