Bug 1600103 Comment 4 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

mail3PaneWindowCommands.js

> -        else
> +        else {
>            return false;
> -
> +        }

I think the else can go. Unconditional return false at this point.

mailContextMenus.js

> +  function checkIsVirtualFolder(folder) {
> +    return folder.flags & Ci.nsMsgFolderFlags.Virtual;

You are using folder.getFlag(Ci.nsMsgFolderFlags.Virtual) for this in folderPane.js. More than one time in the patch so maybe search for folder.flags in new code.


> +        !(folder.flags & Ci.nsMsgFolderFlags.Trash))


Same here

>    }
> -  else
> -  {
> +  else {

Better } else { ?

> +  return(true);

return true;


mailWindowOverlay.js

> +      Components.interfaces.nsISubscribableServer);

Ci.nsISubscribableServer


As noted over irc Compact this folder needs either a plural form in the popup menu or we do it the TB way and just stat Compact. 
NIT One line if formatting with curly braces as discussed too. 

Minor problem: popups under Windows have too many separators because of removed items. If we don't find it in time it is cosmetic and can go into a follow up. 

Didn't find any problems so far. f+ for now but overall good job.
mail3PaneWindowCommands.js

> -        else
> +        else {
>            return false;
> -
> +        }

I think the else can go. Unconditional return false at this point.

mailContextMenus.js

> +  function checkIsVirtualFolder(folder) {
> +    return folder.flags & Ci.nsMsgFolderFlags.Virtual;

You are using folder.getFlag(Ci.nsMsgFolderFlags.Virtual) for this in folderPane.js. More than one time in the patch so maybe search for folder.flags in new code.


> +        !(folder.flags & Ci.nsMsgFolderFlags.Trash))


Same here

>    }
> -  else
> -  {
> +  else {

Better } else { ?

> +  return(true);

return true;


mailWindowOverlay.js

> +      Components.interfaces.nsISubscribableServer);

Ci.nsISubscribableServer


As noted over irc Compact this folder needs either a plural form in the popup menu or we do it the TB way and just state Compact. 
NIT One line if formatting with curly braces as discussed too. 

Minor problem: popups under Windows have too many separators because of removed items. If we don't find it in time it is cosmetic and can go into a follow up. 

Didn't find any problems so far. f+ for now but overall good job.

Back to Bug 1600103 Comment 4