[#5012] wrong trash folder options check in Message.php delete()
Summary wrong trash folder options check in Message.php delete()
Queue IMP
Queue Version 4.1.3
Type Bug
State Resolved
Priority 1. Low
Owners chuck@horde.org, slusarz@horde.org
Requester stephan@stean.ch
Created 2007-02-19 (4889 days ago)
Updated 2007-03-01 (4879 days ago)
Assigned 2007-02-28 (4880 days ago)
Resolved 2007-03-01 (4879 days ago)
Patch No

stephan@stean.ch 2007-02-19 18:57:07
The following line will return a wrong result

if the user forgot to set the trash folder in

the IMP options:

$trash = IMP::folderPref($prefs->getValue('trash_folder'), true);


returns nothing, however folderPref will prepend INBOX,

thus the trash folder will be INBOX, The following check

will not fail, however should:


$use_trash = $prefs->getValue('use_trash');

         if ($use_trash && !$trash) {


and @imap_mail_move will report an error.

Jan Schneider <jan@horde.org> 2007-02-21 23:57:56
No, $prefs->getValue('trash_folder') will return the default value 
from config/prefs.php when the user hasn't set a folder yet, and if 
you haven't broken your system.

stephan@stean.ch 2007-02-24 07:55:46
> No, $prefs->getValue('trash_folder') will return the default value

> from config/prefs.php when the user hasn't set a folder yet, and if

> you haven't broken your system.

I thought about having broken something too. But found nothing, and all the

other settings from prefs.php where readable. So I made a test with a vanilla

installation, with minor changes in servers.php. And came to the same results.

Here what happens:

Mail options "Deleting and Moving messages", I enable the "When 
deleting messages, move..."

option. But dont choose a trash folder from the list. IMP will insert 
an empty trash_folder into the

horde_prefs table.

However, if I enable the very same option, choose "Create a new 
trashfolder", instead of entering

the name press the cancel button, it works. IMP wont insert the empty 
trash_folder into the

horde_prefs table and uses the prefs.php setting.

And of course if giving a name for the trash folder when asked for, 
instead of pressing cancel,

that one will be used by IMP.

So, the problem is quite somewhere else, namely in the page processing 
of the IMP options.

imp/lib/prefs.php  function handlefolders($updated, $pref, $folder, 
$new) does the following:

             if ($folder == IMP_PREF_NO_FOLDER) {

                 $prefs->setValue($pref, '');

             } else {

I think when a user chooses to move deleted messages to a trash folder 
this code fragment

does the wrong job. It should do as in the else part that follows, 
choose an appropriate trash

folder name and use it.

Thats the way I think about it.

Jan Schneider <jan@horde.org> 2007-02-28 18:29:02
I'm really not sure, I can see both actions being the right one. 
Michael, Chuck, what do you think?

Michael Slusarz <slusarz@horde.org> 2007-03-01 07:19:58
Makes sense to only allow move to trash if a trash folder is defined.   
Fixed in HEAD and 4.1.4: