Open Bug 1842514 Opened 1 year ago Updated 1 year ago

Allow folder copy within a server and non-recursive folder transfers within and between servers

Categories

(Thunderbird :: Folder and Message Lists, enhancement)

enhancement

Tracking

(Not tracked)

ASSIGNED

People

(Reporter: gds, Assigned: gds)

References

Details

(Whiteboard: [snnot3p])

Attachments

(2 files, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #1831759 +++

This addresses the issue described in bug 1831759 comment 17 and now allows folder copy within the same server, not just move.

In addition, this adds the ability to do non-recursive folder copies and moves so that an individual folder in the tree can the transferred without transferring all the sub-folders beneath it.

To support non-recursive folder transfers a new pref is added mail.recursiveFolderTransfer which defaults to true and is reflected in the new folder context menu checkbox item Include Subfolders in the in the "Copy/Move" section which will show the "check" when the pref is true -- the default state. If the user wants to only transfer a single folder and not any of that folder's subfolders (if there are any), the the user must un-check the Include Subfolders item. See the attachment.

There is one limitation if the user wants to move an individual folder but not its subfolders within the same server. Since a move within a server does a "pure" move which causes the source folder to be deleted, it would leave any subfolders below it orphaned (with no parent). To avoid the complication of possibly moving the orphaned subfolders up to a different parent, move of folder within a server is always recursive regardless of the context menu selection or the corresponding pref setting.

Non-recursive moves between servers doesn't have this problem because the move is "impure" in that only the messages of the source folder are moved while the source folder remains in place. And, of course, non-recursive folder copies within a server and between servers doesn't have this limitation.

Summary: Add support for folder move and copy from folder context menu → Allow folder copy within a server and non-recursive folder transfers within and between servers

WIP. Need to run a "try" build on this and if OK will submit via moz-phab.

Assignee: nobody → gds

Comment on attachment 9342974 [details] [diff] [review]
allow-folder-copy-inside-server-plus-non-recursive-xfers.diff

Added fix for folder context menu mochitest (one new item added to menu) and submitted diff via moz-phab. (Note: try server seems to be going to "Complete" state immediately so unable to re-submit my fix to try.)

Attachment #9342974 - Attachment is obsolete: true

Comment on attachment 9342972 [details]
New-context-menu-item-INCLUDE-SUBFOLDERS.png

Can we do something better? I don't have any ideas here but this isn't great.

Gene, I think it would be helpful to split your patch into two parts – the UI and everything else. Then at least we can get some traction on them separately without a lot of review comment clutter.

Attachment #9342972 - Flags: ui-review?(alessandro)

I think a natural thing to do would be to always just copy recursively. I don't know any other apps where you could copy just a parent folder without its children. Even if this is kind of possible.

Comment on attachment 9342972 [details]
New-context-menu-item-INCLUDE-SUBFOLDERS.png

Indeed, this is a no go in terms of UI and UX.
It's an interesting idea, and Gene thank you so much for tackling this, your contributions are immensely appreciated.

I think indeed the default behaviour should be the recursive inclusion of all subfolders.
That's how any file system behaves and it would be kinda weird to offer this sort of exception.

Doing so also opens up to the complexity of needing to move child folders up a level as they would orphans.
I agree with Magnus, we shouldn't do this and only handle things recursively.

Attachment #9342972 - Flags: ui-review?(alessandro) → ui-review-

(In reply to Magnus Melin [:mkmelin] from comment #5)

I think a natural thing to do would be to always just copy recursively. I don't know any other apps where you could copy just a parent folder without its children. Even if this is kind of possible.

Yes, I don't see non-recursive copy option in file manager apps I have, but several utilities do. On these you have to include -r to do recursive directory copies, for example:
cp -r
rsync -r
scp -r

I'm thinking that a non-recursive copy option would be useful in TB when you want to copy/move to another server a single folder but don't want to bring along all the subfolders because the subfolders contain a lot of messages that you don't want to wait to be copied. (Bulk copies to imap servers can often be slow and unreliable unlike when copying with an app to local filesystem.) But probably this is pretty rare, so just the user settable pref would be enough to handle this case rather than adding an item to the folder context menu?

Status: NEW → ASSIGNED

(In reply to Alessandro Castellani [:aleca] from comment #6)

I think indeed the default behaviour should be the recursive inclusion of all subfolders.
That's how any file system behaves and it would be kinda weird to offer this sort of exception.

I'm now still suggesting default to be recursive copies/move and only do non-recursive if users sets the proposed new pref mail.recursiveFolderTransfer to false. So this wouldn't appear in the main UI, just in "Advanced Settings...".

Doing so also opens up to the complexity of needing to move child folders up a level as they would orphans.

I addressed this in comment 0:

To avoid the complication of possibly moving the orphaned subfolders up to a different parent, move of folder within a server is always recursive regardless of the context menu selection or the corresponding pref setting.

To avoid the complication of possibly moving the orphaned subfolders up to a different parent, move of folder within a server is always recursive regardless of the context menu selection or the corresponding pref setting.

Already this is a bit of a red flag for me since you're addition an exception to a pref that in itself is already an exception to the default behaviour.
This is indeed a pretty niche feature that could create unpredictable consequences for users that expect a specific behaviour or don't remember changing that pref.

I'd recommend to split this implementation into 2 bugs.

In here we should only tackle the "Allow folder copy within a server" feature, so it's self contained, it doesn't introduce any new string, and if we deemed it safe we can uplift it to 115.

Another enhancement bug should be created to explore if we want the non-recursive feature, and if we do how to properly expose it to users.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: