65995 - "Edit | Properties" should be context specific

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[jglick]

Spin off of bug: 65019

"Edit | Properties" is currently the same regardless of what has focus.

Since we are using context sensitive menu items in the other menus, it would be 
nice if this one was also.  Especially since when a message has focus, the Edit 
"Properties" item refers to the folder/server in the left pane instead of the 
message that is selected.

The "Edit | Properties" menu item should be context specific for the following 
items:  Folder, Mail Server, News Server or Newsgroup Properties.

Comment 1

[Håkan Waara]

OS -> All
Platform -> All

Comment 2

[Jesse Ruderman]

See also bug 67614, need more ways to access properties (Ctrl+I, etc).

Comment 3

[Ben Bucksch (:BenB)]

> The "Edit | Properties" menu item should be context specific for the following
> items:  Folder, Mail Server, News Server or Newsgroup Properties.

And msg.

(Otherwise, the menu item is misplaced and misnamed.)

Comment 4

[jglick]

Nominating this for nsbeta1.

Comment 5

[scottputterman]

marking nsbeta1-

Comment 6

[andreww]

Taking off seth's plate

Comment 7

[Karen Huang]

*** Bug 123096 has been marked as a duplicate of this bug. ***

Comment 8

[Karen Huang]

Please consider this bug for next release (Mach V) since we need this for Folder 
Properties for specifying IMAP Shared Folder.

Comment 9

[Lori Kaplan]

approving nomination. Needed for shared folders feature. 

Comment 10

[jglick]

For clarification:
"Folder Properties" and "Newsgroup Properties" open the 
<Folder/Newsgroup> Properties dialog.

"Mail Server Properties" and "Newsgroup (Not "News") Server Properties" should 
be enabled and open the "Mail & Newsgroups Account Settings" dialog. The top 
level item for the correct account (account with focus as time) should be 
selected.

Comment 11

[jglick]

>"Mail Server Properties" and "Newsgroup (Not "News") Server Properties" should 
>be enabled and open the "Mail & Newsgroups Account Settings" dialog. The top 
>level item for the correct account (account with focus as time) should be 
>selected.

Andrew just mentioned that "Mail & Newsgroup Account Settings" menu item is 
already in the Edit menu. So leaving "Properties" disabled when a server is 
selected is fine.

Comment 12

[andreww]

should have this ready early in 1.0 cycle.

Comment 13

[andreww]

Attachment: changes to mail3PaneWindowCommands.js, messenger.dtd, mailWindowOverlay.xul

patch ready for review

Comment 14

[Håkan Waara]

Comment on attachment 71433 [details] [diff] [review]
changes to mail3PaneWindowCommands.js, messenger.dtd, mailWindowOverlay.xul

>+   var specialFolder = null;
>+   var isServer = null;
>+   var serverType = null;

Please declare these variables inside the try block, which is where you use
them. Also, why do you initialize them to null?

>+       folderOutliner.outlinerBoxObject.selection.getRangeAt(0, startIndex, endIndex);

Bad indentation.

>+        //isServer = GetFolderAttribute(folderOutliner, folderResource, "IsServer");

Is 'isServer' unused, and can be removed?

>+        switch (serverType)
>+        {
>+           case "none":
>+           case "imap":
>+              goSetMenuValue(command, 'valueFolder');
>+              break;

Is it just me, or is there a "pop3" that should be added here as well?

>+      dump("properties menu failure: " + ex + "\n");

This is not needed. An exception will thrown anyway, in the JS console.

Comment 15

[andreww]

great feedback! New patch forthcoming.

Comment 16

[andreww]

Attachment: patch v2 with changes from comments.

try # 2 with changes from Joe's comments.

Comment 17

[Håkan Waara]

Comment on attachment 71713 [details] [diff] [review]
patch v2 with changes from comments.

Looks like this patch is corrupt - there's leading and trailing cruft.

BTW, who's Joe? ;-)

r=hwaara

Comment 18

[andreww]

Goodness! - forgive me Hwaara - I told hewitt to review this and I totally 
didnt see the user name there. I had assumed that he'd looked at it 
already and had posted comments. Thanks for the good feedback all the 
same :)  

Comment 19

[andreww]

Attachment: patch

Comment 20

[(not reading, please use [e-mail] instead)]

Comment on attachment 71744 [details] [diff] [review]
patch

carrying over the r=

Comment 21

[(not reading, please use [e-mail] instead)]

1) I don't see why you need these lines:

+      var specialFolder; 
+      specialFolder = GetFolderAttribute(folderOutliner, 
folderResource, "SpecialFolder");

2)  is commandglue.js already included (I think so.)  you can use 
GetSelectedFolderResource() instead of duplicating the code.

Comment 22

[andreww]

Attachment: changes to mail3panewindowcommands.js - seth's recommendations

update to js file only - per seth's comments.

Comment 23

[(not reading, please use [e-mail] instead)]

Comment on attachment 72071 [details] [diff] [review]
changes to mail3panewindowcommands.js - seth's recommendations

sr=sspitzer

Comment 24

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 72071 [details] [diff] [review]
changes to mail3panewindowcommands.js - seth's recommendations

a=dbaron for trunk checkin (although it's nice to attach the complete patch so
we don't have to piece it together mentally from multiple attachments)

Comment 25

[andreww]

fix check into the trunk

Comment 26

[Olga]

Verified on Win2K, Linux, Mac OSX - 03-04-2002 trunk.
Checked with selected Folder in Mail and in Newsgroup, also with message selected.

Comment 27

[(not reading, please use [e-mail] instead)]

follow up:

in bug #172178, I've made it so it's "Edit | Properties..." and enabled for servers.

and when you use it, it brings up the account manager to that account.