Last Comment Bug 293679 - Hide folder pane context menu items for virtual folders
: Hide folder pane context menu items for virtual folders
Status: VERIFIED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Message Display (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Ian Neal (Away until 7th Aug)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-05-10 14:58 PDT by Ian Neal (Away until 7th Aug)
Modified: 2006-01-06 12:17 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v0.1 (6.25 KB, patch)
2005-05-10 15:01 PDT, Ian Neal (Away until 7th Aug)
neil: review-
Details | Diff | Splinter Review
Revised Patch v0.1a (6.09 KB, patch)
2005-05-12 14:38 PDT, Ian Neal (Away until 7th Aug)
neil: review+
Details | Diff | Splinter Review
Avoid false advertising (873 bytes, patch)
2005-05-13 02:31 PDT, neil@parkwaycc.co.uk
mozilla: review+
mozilla: superreview+
asa: approval1.8b2+
Details | Diff | Splinter Review
Revised Patch v0.1b (Checked in) (4.91 KB, patch)
2005-05-13 10:30 PDT, Ian Neal (Away until 7th Aug)
iann_bugzilla: review+
mozilla: superreview+
asa: approval1.8b2+
Details | Diff | Splinter Review

Description Ian Neal (Away until 7th Aug) 2005-05-10 14:58:10 PDT
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 (Away until 7th Aug) 2005-05-10 15:01:18 PDT
Created attachment 183205 [details] [diff] [review]
Patch v0.1

This patch:
* Hides above menu items for virtual folders
Comment 2 Ian Neal (Away until 7th Aug) 2005-05-11 06:12:27 PDT
Maybe worth checking https://bugzilla.mozilla.org/show_bug.cgi?id=293682#c2
Comment 3 neil@parkwaycc.co.uk 2005-05-12 06:50:10 PDT
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 (Away until 7th Aug) 2005-05-12 14:38:03 PDT
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
Comment 5 neil@parkwaycc.co.uk 2005-05-13 02:31:48 PDT
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.
Comment 6 neil@parkwaycc.co.uk 2005-05-13 03:10:28 PDT
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 neil@parkwaycc.co.uk 2005-05-13 03:21:54 PDT
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 (Away until 7th Aug) 2005-05-13 10:30:34 PDT
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=
Comment 9 Ian Neal (Away until 7th Aug) 2005-05-13 10:47:51 PDT
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] 2005-05-13 11:16:37 PDT
Comment on attachment 183512 [details] [diff] [review]
Revised Patch v0.1b (Checked in)

a=asa
Comment 11 Ian Neal (Away until 7th Aug) 2005-05-13 14:15:24 PDT
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 neil@parkwaycc.co.uk 2005-05-14 23:53:45 PDT
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] 2005-05-19 08:55:59 PDT
Comment on attachment 183481 [details] [diff] [review]
Avoid false advertising

a=asa
Comment 14 neil@parkwaycc.co.uk 2005-05-19 09:27:56 PDT
Fix checked in.
Comment 15 Stephen Donner [:stephend] 2006-01-06 12:17:47 PST
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.

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