Comment 292 for bug 52667

Created an attachment (id=382384)
Disable menu items, with mkmelin's fixes.

(In reply to comment #234)
> This seems to work well, however I have a few nits.
>
> Please capitalize the js method names like it's elsewhere in the file/s.

Fixed.

> >+++ b/mail/base/content/mail3PaneWindowCommands.js
> >+ let retval = false;
> Mozilla code almost always use rv for that.

Fixed.

> > if (GetNumSelectedMessages() > 0)
> > {
> > if (gDBView)
> > {
> > gDBView.getCommandStatus(nsMsgViewCommandType.cmdRequiringMsgBody, enabled, checkStatus);
> >- return enabled.value;
> >+ retval = enabled.value;
> > }
> > }
> >- return false;
> >+ if (retval)
> >+ {
> >+ if (command == "cmd_reply" || command == "button_reply")
> >+ retval = isReplyEnabled();
> >+ else if (command == "cmd_replyall" || command == "button_replyall")
> >+ retval = isReplyAllEnabled();
> >+ else if (command == "cmd_replylist" || command == "button_replylist")
> >+ retval = isReplyListEnabled();
> >+ }
> Why aren't these higher up directly after each "case foo:"?

Because I only want to check them if
"gDBView.getCommandStatus(nsMsgViewCommandType.cmdRequiringMsgBody,
enabled, checkStatus);" returns true, and I didn't want to repeat the whole
"if (GetNumSelectedMessages() > 0)" block four times. (If there's a better
way to do this, I'ld love to change to it, but I couldn't think of one.)

> >+++ b/mail/base/content/mailWindowOverlay.js
> >@@ -807,10 +807,45 @@
> >+function getServerType()
> Maybe name the method GetCurrentMsgServerType or something more descriptive?

I've changed it to "GetLoadedMsgServerType", since I'm now using
"GetLoadedMsgFolder" (because it was doing the same thing I was).

> >+ if (getServerType() == "rss")
> >+ // If we're in an rss item, we never want to Reply, because there's
> >+ // usually no-one useful to reply to.
> >+ return false;
> >+ return true;
> Could just be |return getServerType() != "rss";|

Fixed.

Thanks,
Blake.