Disable File | Subscribe menu when only POP account is available

RESOLVED FIXED in seamonkey2.42

Status

SeaMonkey
MailNews: Message Display
--
minor
RESOLVED FIXED
17 years ago
2 years ago

People

(Reporter: ji, Assigned: aceman)

Tracking

Trunk
seamonkey2.42

SeaMonkey Tracking Flags

(seamonkey2.40 affected, seamonkey2.41 affected, seamonkey2.42 affected)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

17 years ago
With the latest NS6.1 build, File | Subscribe menu is not disabled when there is
only POP account is available. In this case, selecting File | Subscribe menu
will bring up an unusable subscribe window with no accounts and no folders on
it. Since subscrible is for IMAP and news account, we should disable subscrible
menu in this case.

Steps to reproduce:
1. Launch mail with a new profile.
2. Set up a POP account with this profile.
3. On Mail window, select File | Subscribe
accepting.
Status: NEW → ASSIGNED

Updated

14 years ago
Blocks: 236667
Product: Browser → Seamonkey

Updated

13 years ago
Assignee: sspitzer → mail
Status: ASSIGNED → NEW

Updated

10 years ago
Component: MailNews: Subscribe → MailNews: Message Display
QA Contact: stephend → search
Assignee: mail → nobody
QA Contact: search → message-display
This bug is being marked EXPIRED as it has seen no activity in a very long time.

If you think that the issue reported might still be relevant, please test with a recent release of SeaMonkey and if the problem persists feel free to re-open the report. Thank you.

http://www.seamonkey-project.org/
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → EXPIRED
Bulk reopening incorrectly expired bugs - no activity does not constitute no bug - these need proper checking.
Status: RESOLVED → REOPENED
Resolution: EXPIRED → ---
Status: REOPENED → UNCONFIRMED
Confirming: Still an issue with current trunk (SM 2.1b2pre). Note that we also have Feed accounts now, for which File|Subscribe works, too.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

7 years ago
Severity: normal → minor
(Assignee)

Comment 5

4 years ago
Created attachment 8498455 [details] [diff] [review]
patch for TB

This is a version of the fix for Thunderbird.
As this bug is for SM, maybe you'd like to adapt it to SM too.
Attachment #8498455 - Flags: review?(mkmelin+mozilla)
Attachment #8498455 - Flags: feedback?(iann_bugzilla)

Comment 6

4 years ago
Comment on attachment 8498455 [details] [diff] [review]
patch for TB

Review of attachment 8498455 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, r=mkmelin
Attachment #8498455 - Flags: review?(mkmelin+mozilla) → review+
(Assignee)

Comment 7

3 years ago
The patch seems to still apply. Let's get it landed so it is not lost.
Keywords: checkin-needed
Whiteboard: [keep open for SM part]

Updated

3 years ago
Assignee: nobody → acelists
Status: NEW → ASSIGNED
(Assignee)

Comment 8

3 years ago
rsx11m, I did not assign this to me as I do not intend to do the Seamonkey version as I can't test it. But if you do, I can prepare it.
(Assignee)

Comment 9

3 years ago
(In reply to :aceman from comment #8)
> rsx11m, I did not assign this to me as I do not intend to do the Seamonkey
> version as I can't test it. But if you do, I can prepare it.

I mean whether you can test it if I prepare the patch.
Flags: needinfo?(rsx11m.pub)

Comment 10

3 years ago
Sure, I can test the SM version of the patch. Just ask me for feedback on it, though I don't have a "real" POP account (but I can set one up to verify if the menu item is hidden).

And sorry, I've missed your comment #8.
Flags: needinfo?(rsx11m.pub)
(Assignee)

Comment 11

3 years ago
Created attachment 8672328 [details] [diff] [review]
patch for SM
Attachment #8672328 - Flags: feedback?(rsx11m.pub)
(Assignee)

Comment 12

3 years ago
(In reply to rsx11m from comment #10)
> Sure, I can test the SM version of the patch. Just ask me for feedback on
> it, though I don't have a "real" POP account (but I can set one up to verify
> if the menu item is hidden).

Actually this patch is not about the context menu on account/folder, but about the main menu item. It should get disabled if there are no IMAP/News accounts existing and you also have not a RSS account selected.

Comment 13

3 years ago
Comment on attachment 8672328 [details] [diff] [review]
patch for SM

> +  let servers = MailServices.accounts.allServers;
> +  for each (let server in fixIterator(servers,
> +                                      Components.interfaces.nsIMsgIncomingServer)) {
1. We aren't supposed to use for-each any more. Please use for-off instead.
2. Ci shortcut isn't available here. Use Components.interfaces.nsIMsgIncomingServer instead.

> for (let server of fixIterator(MailServices.accounts.allServers,
>                                Components.interfaces.nsIMsgIncomingServer)) {
Flags: needinfo?(acelists)
(Assignee)

Comment 14

3 years ago
I do not see Ci. being used anywhere in the patch... Thanks for the for..of hint, support for that (with fixIterator) was only added recently so I am not used to it ;)
Flags: needinfo?(acelists)
(Assignee)

Comment 15

3 years ago
Created attachment 8674447 [details] [diff] [review]
patch for SM v1.1
Attachment #8672328 - Attachment is obsolete: true
Attachment #8672328 - Flags: feedback?(rsx11m.pub)
Attachment #8674447 - Flags: feedback?(rsx11m.pub)
(Assignee)

Comment 16

3 years ago
Created attachment 8674448 [details] [diff] [review]
patch for TB v1.1
Attachment #8498455 - Attachment is obsolete: true
Attachment #8498455 - Flags: feedback?(iann_bugzilla)
Attachment #8674448 - Flags: review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
Whiteboard: [keep open for SM part]

Comment 17

3 years ago
Comment on attachment 8674447 [details] [diff] [review]
patch for SM v1.1

Ping feedback request?
Flags: needinfo?(rsx11m.pub)
Attachment #8674447 - Flags: review?(iann_bugzilla)

Comment 18

3 years ago
Sorry, I've been rather busy in real life, but hope to get to it soon.
Flags: needinfo?(rsx11m.pub)

Comment 19

3 years ago
Comment on attachment 8674447 [details] [diff] [review]
patch for SM v1.1

>+++ b/suite/mailnews/mail3PaneWindowCommands.js
>@@ -150,16 +150,17 @@ var DefaultController =
>       case "cmd_viewIgnoredThreads":
>       case "cmd_stop":
>       case "cmd_undo":
>       case "cmd_redo":
> 			case "cmd_expandAllThreads":
> 			case "cmd_collapseAllThreads":
> 			case "cmd_renameFolder":
> 			case "cmd_sendUnsentMsgs":
>+			case "cmd_subscribe":
Please line up with case "cmd_redo" and case "button_print" (no tabs)
> 			case "cmd_openMessage":
>       case "button_print":

>@@ -589,16 +592,19 @@ var DefaultController =

>+			case "cmd_subscribe":
>+				MsgSubscribe();
>+				return;
Please line up with/indent like case "cmd_printSetup" (no tabs)
> 			case "cmd_openMessage":
>                 MsgOpenSelectedMessages();
> 				return;
>             case "cmd_printSetup":
>                 PrintUtils.showPageSetup();
>                 return;

>@@ -844,16 +850,39 @@ function IsSendUnsentMsgsEnabled(folderR

>+/**
>+ * Determine whether there exists any server for which to show the Subscribe dialog.
>+ */
>+function IsSubscribeEnabled()
>+{
>+  // If there are any IMAP or News servers, we can show the dialog any time and
>+  // it will properly show those.
>+  let servers = MailServices.accounts.allServers;
SM doesn't use MailServices yet in this file, accountManager.allServers should work (see http://mxr.mozilla.org/comm-central/source/suite/mailnews/mail3PaneWindowCommands.js#1093)

r=me with those addressed.
Attachment #8674447 - Flags: review?(iann_bugzilla) → review+

Comment 20

3 years ago
Comment on attachment 8674447 [details] [diff] [review]
patch for SM v1.1

Works fine for me out of the box with a recent 2.41a1 nightly build. The File > Subscribe menuitem isn't visible with a new profile or after creating any number of POP accounts. It is only shown after creating a Newsgroup or IMAP account, or both. Removing those accounts subsequently disables the menuitem again.

>+++ b/suite/mailnews/mail3PaneWindowCommands.js
>+function IsSubscribeEnabled()
>+{
>+  // If there are any IMAP or News servers, we can show the dialog any time and
>+  // it will properly show those.
>+  let servers = MailServices.accounts.allServers;

I didn't need to apply the change suggested in comment #19, thus it appears that MailServices is defined or included somewhere/somehow already. I therefore didn't test

>-  let servers = MailServices.accounts.allServers;
>+  let servers = accountManager.allServers;

as an alternate patch.
Attachment #8674447 - Flags: feedback?(rsx11m.pub) → feedback+

Comment 21

3 years ago
Setting status flags as SeaMonkey probably want this on the branches during the upcoming cycle.
status-seamonkey2.40: --- → affected
status-seamonkey2.41: --- → affected
status-seamonkey2.42: --- → affected
(Assignee)

Comment 22

3 years ago
Created attachment 8687643 [details] [diff] [review]
patch for SM v1.2

Fixed problems from comment 19, thanks.
Attachment #8674447 - Attachment is obsolete: true
Attachment #8687643 - Flags: review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed

Comment 23

2 years ago
Comment on attachment 8687643 [details] [diff] [review]
patch for SM v1.2

a=Ratty for SeaMonkey CLOSED TREE
(Assignee)

Comment 24

2 years ago
https://hg.mozilla.org/comm-central/rev/075959e1c604
https://hg.mozilla.org/comm-central/rev/f99e915e6dac
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.42
You need to log in before you can comment on or make changes to this bug.