293679 - Hide folder pane context menu items for virtual folders

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Ian Neal]

There are various folder pane context menu items that need to be hidden:
a) Copy Folder Location
b) Subscribe
c) Mark all Read
d) Compact This Folder
e) Search Messages

Equivalent TB bugs - bug 266578 and bug 266674

Comment 1

[Ian Neal]

Attachment: Patch v0.1

This patch:
* Hides above menu items for virtual folders

Comment 2

[Ian Neal]

Maybe worth checking https://bugzilla.mozilla.org/show_bug.cgi?id=293682#c2

Comment 3

[[e-mail]]

Comment on attachment 183205 [details] [diff] [review]
Patch v0.1

>+  var folder = GetMsgFolderFromUri(folderResource.Value, false);
>+  var isVirtualFolder = folder ? folder.flags & MSG_FOLDER_FLAG_VIRTUAL : false;
What's wrong with using specialFolder == "Virtual" ?

>-  ShowMenuItem("folderPaneContext-subscribe", (numSelected <= 1) && canSubscribeToFolder);
>-  EnableMenuItem("folderPaneContext-subscribe", true);
>+  ShowMenuItem("folderPaneContext-subscribe", (numSelected <= 1) && canSubscribeToFolder && !isVirtualFolder);
>+  EnableMenuItem("folderPaneContext-subscribe", !isVirtualFolder);
Don't bother disabling hidden items.

>-  ShowMenuItem("folderPaneContext-searchMessages", (numSelected<=1));
>+  ShowMenuItem("folderPaneContext-searchMessages", (numSelected<=1) && !isVirtualFolder);
Might as well fix the spacing around <= while you're at it.

> function SetupCompactMenuItem(folderResource, numSelected)
Need to pass specialFolder in here (c.f. Rename, Remove).

> function IsCanSearchMessagesEnabled()
> {
>   var folderURI = GetSelectedFolderURI();
>   var server = GetServer(folderURI);
>-  return server.canSearchMessages;
>+  var folder = GetMsgFolderFromUri(folderURI, false);
>+  var isVirtualFolder = folder ? folder.flags & MSG_FOLDER_FLAG_VIRTUAL : false;
>+  return server.canSearchMessages && !isVirtualFolder;
> }
> function IsFolderCharsetEnabled()
I think we can assume that the folder exists - it's not null-checking server or
anything like that. You might as well inline the appropriate portion of the
GetServer function call to save you from fetching the folder twice. Also stick
in a separator line before the next function while you're here?

Comment 4

[Ian Neal]

Attachment: Revised Patch v0.1a

Changes since v0.1 (as per review comments):
* Switched to using SpecialFolder == "Virtual"
* Stopped disabling hidden items
* Fixed up spacing round <=
* Passed isVirtualFolder through to SetupCompactMenuItem function
* Stopped getting folder twice in IsCanSearchMessagesEnabled function

Comment 5

[[e-mail]]

Attachment: Avoid false advertising

It seems to me that it's less ugly doing it here than working around it in XUL.

Comment 6

[[e-mail]]

Comment on attachment 183438 [details] [diff] [review]
Revised Patch v0.1a

When testing this I found it really hard to add a search folder. Perhaps there
should be a File/New option...

Comment 7

[[e-mail]]

Comment on attachment 183438 [details] [diff] [review]
Revised Patch v0.1a

Bah, clicking on a virtual folder doesn't work when a server was previously
selected :-/

Comment 8

[Ian Neal]

Attachment: Revised Patch v0.1b (Checked in)

Changes since v0.1a:
* Removed canCompact bits as they have superceeded by Neil's patch

Carrying forward r= and requesting sr=

Comment 9

[Ian Neal]

Comment on attachment 183512 [details] [diff] [review]
Revised Patch v0.1b (Checked in)

Requesting a= for low risk, suite-only patch

Comment 10

[Asa Dotzler [:asa]]

Comment on attachment 183512 [details] [diff] [review]
Revised Patch v0.1b (Checked in)

a=asa

Comment 11

[Ian Neal]

Comment on attachment 183512 [details] [diff] [review]
Revised Patch v0.1b (Checked in)

Checking in mailContextMenus.js;
new revision: 1.55; previous revision: 1.54
mail3PaneWindowCommands.js;
new revision: 1.144; previous revision: 1.143
done

Comment 12

[[e-mail]]

Comment on attachment 183481 [details] [diff] [review]
Avoid false advertising

Trivial patch to complete this bug in a simpler way than the original patch.

Comment 13

[Asa Dotzler [:asa]]

Comment on attachment 183481 [details] [diff] [review]
Avoid false advertising

a=asa

Comment 14

[[e-mail]]

Fix checked in.

Comment 15

[Stephen Donner [:stephend] Not actively reading bugmail]

Context menu now appears as:

Open in New Mail Window
-----------------------
Rename Folder
Delete Folder
-----------------------
Properties

Verified FIXED using build 2006-01-06-09 of SeaMonkey trunk under Windows XP.