346747 - Add Rename and Delete Support to Folder Views

Status: NEW

(Thunderbird :: General, defect)

Description

[Scott MacGregor]

Currently, if you rename or delete a folder while within a view like favorite folders, you have to change the folder view to something else, then cycle back in order to see the renamed folder.

Comment 1

[Scott MacGregor]

my fix for this works best with the fix for Bug 346378

Comment 2

[Scott MacGregor]

Attachment: fix for renaming

This change fixes rename and lays the foundation for folder deletion from within a view like favorites.

When adding and removing items after the datasource was built the template builder was never re-building with the new items. The main fix is explained here:

+nsresult nsMsgFlatFolderDataSource::OnItemAddedOrRemoved(nsIRDFResource *parentItem, nsISupports *item, PRBool added)
 {
-  const char *uri;
-  nsCAutoString dsUri(m_dsName);
-  dsUri.Append(":/");
+  // When a folder is added or removed, parentItem is the parent folder and item is the folder being
+  // added or removed. In a flat data source, there is no arc in the graph between the parent folder
+  // and the folder being added or removed. Our flat data source root (i.e. mailnewsunreadfolders:/) has 
+  // an arc with the child property to every folder in the data source.  We must change parentItem
+  // to be our data source root before calling nsMsgFolderDataSource::OnItemAddedOrRemoved. This ensures
+  // that datasource listeners such as the template builder properly handle add and remove 
+  // notifications on the flat datasource.
+  return nsMsgFolderDataSource::OnItemAddedOrRemoved(m_rootResource, item, added);
+}

With this patch you can now rename folders in a folder view without having to cycle away and back in order to actually see the renamed item.

Comment 3

[Scott MacGregor]

Comment on attachment 231517 [details] [diff] [review]
fix for renaming

Rename just failed for me after renaming a folder 4 or 5 times. This patch still isn't interacting right with the change to Bug 346747.

Comment 4

[Scott MacGregor]

Attachment: [fixed branch and trunk] updated fix

this tweaks how we set the flags on the newfolder during the rename operation. I was getting into scenarios where the data source was getting told of the addition in OnItemAdded, but the favorites flag hadn't been set on newfolder in nsLocalMailFolder::Rename yet.

Note: rename for most recent folders doesn't work yet, but rename works in the rest of the views now.

Comment 5

[David :Bienvenu]

Comment on attachment 231521 [details] [diff] [review]
[fixed branch and trunk] updated fix

that worked for me.

Comment 6

[Scott MacGregor]

Comment on attachment 231521 [details] [diff] [review]
[fixed branch and trunk] updated fix

leaving open for some of the remaining issues with delete and with rename from  the recent folders view.

Comment 7

[Scott MacGregor]

Attachment: [fixed branch and trunk] fix for deleting local folders from the favorites view

This seemed like the best spot to do this, let me know if you disagree. When deleting a local folder to the Trash folder, we want to clear the favorite folder flag otherwise the folder stays in the favorites view because th folder still exists (under Trash) and it still has the favorites property set on it.

Oddly enough, the favorites flag is already getting cleared when deleting an imap folder to the imap trash folder. I need to understand why that is.

Comment 8

[David :Bienvenu]

Comment on attachment 231528 [details] [diff] [review]
[fixed branch and trunk] fix for deleting local folders from the favorites view

yes, that looks good.

IMAP case is async, which might be why the favorite flag is getting cleared.

Comment 9

[David :Bienvenu]

rename and delete seem to be working for me on the trunk in folder views - should we close this, or are there still outstanding issues?

Comment 10

[Georges-Etienne Legendre]

Unflagging a favorite folder when we're in this view doesn't work too. We have to cycle to refresh the view.

Comment 11

[Wayne Mery (:wsmwk)]

David, close this one out?  STM other bugs cover issues like comment 10.