Closed Bug 293679 Opened 19 years ago Closed 19 years ago

Hide folder pane context menu items for virtual folders

Categories

(SeaMonkey :: MailNews: Message Display, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: iannbugzilla, Assigned: iannbugzilla)

Details

Attachments

(2 files, 2 obsolete files)

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
Attached patch Patch v0.1 (obsolete) — Splinter Review
This patch:
* Hides above menu items for virtual folders
Assignee: mail → bugzilla
Status: NEW → ASSIGNED
Attachment #183205 - Flags: review?(neil.parkwaycc.co.uk)
Maybe worth checking https://bugzilla.mozilla.org/show_bug.cgi?id=293682#c2
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?
Attachment #183205 - Flags: review?(neil.parkwaycc.co.uk) → review-
Attached patch Revised Patch v0.1a (obsolete) — Splinter Review
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
Attachment #183205 - Attachment is obsolete: true
Attachment #183438 - Flags: review?(neil.parkwaycc.co.uk)
It seems to me that it's less ugly doing it here than working around it in XUL.
Attachment #183481 - Flags: superreview?(bienvenu)
Attachment #183481 - Flags: review?(bienvenu)
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 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 :-/
Attachment #183438 - Flags: review?(neil.parkwaycc.co.uk) → review+
Attachment #183481 - Flags: review?(bienvenu) → review+
Changes since v0.1a:
* Removed canCompact bits as they have superceeded by Neil's patch

Carrying forward r= and requesting sr=
Attachment #183438 - Attachment is obsolete: true
Attachment #183512 - Flags: superreview?(bienvenu)
Attachment #183512 - Flags: review+
Attachment #183512 - Flags: superreview?(bienvenu) → superreview+
Comment on attachment 183512 [details] [diff] [review]
Revised Patch v0.1b (Checked in)

Requesting a= for low risk, suite-only patch
Attachment #183512 - Flags: approval1.8b2?
Comment on attachment 183512 [details] [diff] [review]
Revised Patch v0.1b (Checked in)

a=asa
Attachment #183512 - Flags: approval1.8b2? → approval1.8b2+
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
Attachment #183512 - Attachment description: Revised Patch v0.1b → Revised Patch v0.1b (Checked in)
Attachment #183481 - Flags: superreview?(bienvenu) → superreview+
Comment on attachment 183481 [details] [diff] [review]
Avoid false advertising

Trivial patch to complete this bug in a simpler way than the original patch.
Attachment #183481 - Flags: approval1.8b2?
Comment on attachment 183481 [details] [diff] [review]
Avoid false advertising

a=asa
Attachment #183481 - Flags: approval1.8b2? → approval1.8b2+
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: