Last Comment Bug 369393 - Remove "Copy Folder Location" from menu -- put the data in Properties dialog
: Remove "Copy Folder Location" from menu -- put the data in Properties dialog
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Mail Window Front End (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: Thunderbird 3
Assigned To: Magnus Melin
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-02-05 13:21 PST by Mike Cowperthwaite
Modified: 2010-06-27 09:47 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
screenshot / sharing tab (19.83 KB, image/png)
2007-07-29 10:02 PDT, Magnus Melin
no flags Details
proposed fix (16.75 KB, patch)
2007-07-29 10:26 PDT, Magnus Melin
mscott: review+
mscott: superreview+
Details | Diff | Splinter Review

Description Mike Cowperthwaite 2007-02-05 13:21:19 PST
I'm not sure if there is any kind of regular usage for the Copy Folder Location feature.  I personally have no need for it -- hardly anything support mailbox or imap URLs.  The news: URL might have some use, but it's limited.

Even if there's a usage I'm unfamiliar with, I daresay it's not one that's known or understood by the Thunderbird target user.

In order to clean up the context menus on the folders a bit, I suggest removing the menu item, and replacing the feature by adding a copy-able text field to the Properties dialog for each folder.
Comment 1 Magnus Melin 2007-07-25 13:26:25 PDT
Seems it was added for use with shared folders, in bug 112105. Given it's use case, maybe the url field should be under the Sharing tab.
Comment 2 WADA 2007-07-26 00:56:44 PDT
Same enhancement request as App Suite Bug 180546.
Comment 3 Magnus Melin 2007-07-29 10:02:10 PDT
Created attachment 274364 [details]
screenshot / sharing tab

Here is a screenshot of what it could look like under the sharing tab.
Comment 4 Magnus Melin 2007-07-29 10:26:02 PDT
Created attachment 274367 [details] [diff] [review]
proposed fix

Removes the "Copy Folder Location" from the context menu, and adds it under the Sharing tab of the folder properties as shown in the screenshot of attachment 274364 [details].

(The field is disabled, though my theme doesn't seem to support showing it as such:( )
Comment 5 Magnus Melin 2007-07-29 10:29:48 PDT
Err, meant readonly.

As for bug 180546, I'm not sure this helps much for that...
Comment 6 Scott MacGregor 2007-07-29 12:12:50 PDT
Comment on attachment 274367 [details] [diff] [review]
proposed fix

awesome!
Comment 7 Magnus Melin 2007-07-29 12:52:33 PDT
Checking in mail/base/content/mailContextMenus.js;
/cvsroot/mozilla/mail/base/content/mailContextMenus.js,v  <--  mailContextMenus.js
new revision: 1.25; previous revision: 1.24
done
Checking in mail/base/content/mailWindowOverlay.xul;
/cvsroot/mozilla/mail/base/content/mailWindowOverlay.xul,v  <--  mailWindowOverlay.xul
new revision: 1.216; previous revision: 1.215
done
Checking in mail/locales/en-US/chrome/messenger/folderProps.dtd;
/cvsroot/mozilla/mail/locales/en-US/chrome/messenger/folderProps.dtd,v  <--  folderProps.dtd
new revision: 1.7; previous revision: 1.6
done
Checking in mail/locales/en-US/chrome/messenger/messenger.dtd;
/cvsroot/mozilla/mail/locales/en-US/chrome/messenger/messenger.dtd,v  <--  messenger.dtd
new revision: 1.65; previous revision: 1.64
done
Checking in mailnews/base/resources/content/folderProps.js;
/cvsroot/mozilla/mailnews/base/resources/content/folderProps.js,v  <--  folderProps.js
new revision: 1.27; previous revision: 1.26
done
Checking in mailnews/base/resources/content/folderProps.xul;
/cvsroot/mozilla/mailnews/base/resources/content/folderProps.xul,v  <--  folderProps.xul
new revision: 1.27; previous revision: 1.26
done
Checking in mailnews/base/resources/content/mailContextMenus.js;
/cvsroot/mozilla/mailnews/base/resources/content/mailContextMenus.js,v  <--  mailContextMenus.js
new revision: 1.62; previous revision: 1.61
done
Checking in mailnews/base/resources/content/mailWindowOverlay.xul;
/cvsroot/mozilla/mailnews/base/resources/content/mailWindowOverlay.xul,v  <--  mailWindowOverlay.xul
new revision: 1.333; previous revision: 1.332
done
Checking in suite/locales/en-US/chrome/mailnews/folderProps.dtd;
/cvsroot/mozilla/suite/locales/en-US/chrome/mailnews/folderProps.dtd,v  <--  folderProps.dtd
new revision: 1.16; previous revision: 1.15
done
Checking in suite/locales/en-US/chrome/mailnews/messenger.dtd;
/cvsroot/mozilla/suite/locales/en-US/chrome/mailnews/messenger.dtd,v  <--  messenger.dtd
new revision: 1.217; previous revision: 1.216
done
Comment 8 Karsten Düsterloh 2007-07-29 13:52:42 PDT
Magnus:
It's no friendly behaviour to change SeaMonkey UI without any reviews of any SM person. :-(
Comment 9 Bruno 'Aqualon' Escherl 2007-07-29 14:20:04 PDT
This patch affects POP3 users, as there is no sharing tab in folder properties, so no way to see the folder location.

(In reply to comment #5)
> As for bug 180546, I'm not sure this helps much for that...
Adding a field with the folder location below name in properties' General Information tab would fix that bug and give POP3 users the possibility to see the location of the selected folder.
Comment 10 Karsten Düsterloh 2007-07-29 14:28:35 PDT
Magnus: 
I quite like dropping the menu item, this functionality is too obscure to be in such a prominent place. But showing this info for IMAP only now is not okay.

For SM, I'd like to see the Location box on the "General Information" tab, below the Name box, for all kinds of folders. (This would fix bug 180546, too, so you might want to take that. ;-).
Comment 11 Magnus Melin 2007-07-29 22:06:46 PDT
Sorry about that Karsten, I'll keep it in mind next time! We can take it in bug 180546... (The url only reveals the real file name for pop, it's not the same for imap.)
Comment 12 Magnus Melin 2007-07-29 22:20:33 PDT
I actually tried to put it under name at first, but that's some scary information to show to users who would nearly never care. 

Scott, speak up if you would like it in General Information (under Name) too.
Comment 13 Magnus Melin 2007-07-30 09:38:42 PDT
Doh, folderProps.xul is shared code, so changing it for SeaMonkey only would be hacky:( 

How about maybe leaving it as it is, and going for bug 158023 if I can figure it out? 
Comment 14 Karsten Düsterloh 2007-07-30 23:36:32 PDT
I'm not convinced that these two li'l files are worth sharing, we probably should disentangle such minor UI parts - I filed bug 390262 on that.
We could "get away" with ifdefing the relevant parts here, but that'd be truly hacky indeed.
The thing is just that the path information is not necessarily "scary" for the usual SM audience and it's not shown prominently enough to "scare" more "basic" users - you rarely open that dialog anyway. Furthermore, the path information is useful for non-IMAP (like pop3 or movemail) as well, especially on unixoid systems where the path almost directly relates to a real path on disk.
So if TB doesn't like to have that information on the general page, it's probably best to fork this small dialog.
Comment 15 WADA 2007-08-01 23:30:27 PDT
Re-opening.
Test result with Tb trunk 2007/7/31 build and 2007/8/01 build(MS Win-XP).
 1. "Show file locaition" doesn't exist in context menu of local mail folder
 2. No file name related information in property of local mail folder
Apparently change for 'Remove "Copy Folder Location" from menu' part only.
No change for "put the data in Properties dialog" part.

Why FIXED even though this status?
"put the data in Properties dialog" part is handled by other bug?
If so, why no pointer to the bug for it?
Comment 16 Magnus Melin 2007-08-02 08:06:37 PDT
WADA: I never said this would fix bug 180546. What was checked in removed it from the context menu, and put the info under the Sharing tab (see the screenshot) in the folder properties. 

Indeed, that info is not accessible for non-imap folders now since they do not have the sharing tab. Didn't think anyone would care for that info other than for IMAP folder sharing (not counting debugging as a normal use activity).
Comment 17 WADA 2007-08-02 14:21:51 PDT
(In reply to comment #16)
> WADA: I never said this would fix bug 180546.
Bug 180546 is for App Suite, and this bug is for Thunderbird, because UI is involved. And, although Bug 180546 refers to file name of local mail folders only, our expectation is common solution(enhancement) for local mail folders and IMAP mail folders and newsgroup, and common solution(enhancement) among all mailers of Mozilla family. Further, there are two kind of information related to a folder - (1) file path, (2) internal mail folder path(used in message filter etc.) when local mail folder. These are not required in daily use, but required when trouble shooting(not debugging). 

Magnus, should we open 1(remove "copy folder location") + 3(local folder, IMAP, news)x2(file path, internal URL)x2(Tb, App Suite) bugs separately?  
Comment 18 Magnus Melin 2007-08-03 09:46:28 PDT
Shouldn't be necessary. Fixing the file name stuff would be a side effect, depending on the implementation. And currently the other stuff is not fixable for one app only as the UI code for it is shared.
Comment 19 Nelson Bolyard (seldom reads bugmail) 2007-08-25 11:59:33 PDT
Damn!  I use "Copy Folder Location" daily, or I did until it was removed!
This is a regression, first class!  We've got to stop taking away features 
because one or two people don't use them!
Comment 20 Nelson Bolyard (seldom reads bugmail) 2007-08-25 12:05:08 PDT
The stuff now in folder properties is NOT equivalent to "Copy Folder Location".
I need a way to get a string like 
"news://news.mozilla.org/mozilla.dev.apps.thunderbird"
I used to be able to do it with two clicks and two mouse movements. 
Now, it's not available at all.  
Comment 21 WADA 2007-08-25 14:40:41 PDT
(In reply to comment #19)
> Damn!  I use "Copy Folder Location" daily, or I did until it was removed!
> This is a regression, first class!

If you are talking about Seamonkey's trunk, see bug 180546. We did't/don't request removal of "Copy Folder Location" in see bug 180546. We requested new "URI or file path in property" only for Seamonkey.
Comment 22 Nelson Bolyard (seldom reads bugmail) 2007-08-25 15:00:06 PDT
That fact that bug 180546 did not request the removal of 
"Copy Folder Location" from the menu is irrelevant to this regression.
THIS bug (bug 369393) clearly DID request and DID remove it!
Comment 23 Nelson Bolyard (seldom reads bugmail) 2007-09-01 12:23:11 PDT
The patch for this bug evidently affected core code, common to TB and SM.
In introduced a regression into SM.  I think it is the duty of people
who patch common code, for the purpose of changing one of the two products
(TB or SM) to ensure that it does not cause regressions for the other 
product, by patching the other product if necessary, to avoid regressing
that product.  
Comment 24 Karsten Düsterloh 2007-09-02 05:50:42 PDT
Nelson, I was involved as you can see in comment #14.
Let's take this into your new bug 394578.
Comment 25 Magnus Melin 2007-09-16 08:48:57 PDT
Marking FIXED again... adjustments in bug 180546.

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