Hide folder pane context menu items for virtual folders

VERIFIED FIXED

Status

SeaMonkey
MailNews: Message Display
VERIFIED FIXED
13 years ago
12 years ago

People

(Reporter: Ian Neal, Assigned: Ian Neal)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

13 years ago
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
(Assignee)

Comment 1

13 years ago
Created attachment 183205 [details] [diff] [review]
Patch v0.1

This patch:
* Hides above menu items for virtual folders
Assignee: mail → bugzilla
Status: NEW → ASSIGNED
Attachment #183205 - Flags: review?(neil.parkwaycc.co.uk)
(Assignee)

Comment 2

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

Comment 3

13 years ago
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-
(Assignee)

Comment 4

13 years ago
Created attachment 183438 [details] [diff] [review]
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
Attachment #183205 - Attachment is obsolete: true
Attachment #183438 - Flags: review?(neil.parkwaycc.co.uk)

Comment 5

13 years ago
Created attachment 183481 [details] [diff] [review]
Avoid false advertising

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 6

13 years ago
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

13 years ago
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 :-/

Updated

13 years ago
Attachment #183438 - Flags: review?(neil.parkwaycc.co.uk) → review+

Updated

13 years ago
Attachment #183481 - Flags: review?(bienvenu) → review+
(Assignee)

Comment 8

13 years ago
Created attachment 183512 [details] [diff] [review]
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=
Attachment #183438 - Attachment is obsolete: true
Attachment #183512 - Flags: superreview?(bienvenu)
Attachment #183512 - Flags: review+

Updated

13 years ago
Attachment #183512 - Flags: superreview?(bienvenu) → superreview+
(Assignee)

Comment 9

13 years ago
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 10

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

a=asa
Attachment #183512 - Flags: approval1.8b2? → approval1.8b2+
(Assignee)

Comment 11

13 years ago
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)

Updated

12 years ago
Attachment #183481 - Flags: superreview?(bienvenu) → superreview+

Comment 12

12 years ago
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 13

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

a=asa
Attachment #183481 - Flags: approval1.8b2? → approval1.8b2+

Comment 14

12 years ago
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 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.