Closed Bug 1600103 Opened 1 year ago Closed 2 months ago

Port |Bug 296655 - selection of multiple mailboxes/folders in the folders pane is possible but not usable| to SeaMonkey

Categories

(SeaMonkey :: MailNews: Message Display, task)

task
Not set
normal

Tracking

(seamonkey2.49esr wontfix, seamonkey2.53+ fixed, seamonkey2.57esr? affected)

RESOLVED FIXED
seamonkey 2.82
Tracking Status
seamonkey2.49esr --- wontfix
seamonkey2.53 + fixed
seamonkey2.57esr ? affected

People

(Reporter: iann_bugzilla, Assigned: iann_bugzilla)

References

(Blocks 1 open bug)

Details

(Whiteboard: SM2.53.6)

Attachments

(1 file, 7 obsolete files)

Port the relevant changes from:

  • Bug 296655 - selection of multiple mailboxes/folders in the folders pane is possible but not usable
  • Bug 236667 - hide 'Subscribe...' context menu item when not on an account of the proper server type
  • Bug 1210071 - Fix folder contextmenu regression in Bug 236667
  • Bug 39121 - Unspecialize Trash folder for IMAP "Mark as Deleted" & "Remove Immediately" modes
  • Bug 490326 - don't allow sub-folders or saved searches under smart folders inbox
  • Bug 492964 - Get New Messages for <selected account> is trying to get messages for all the accounts when using global inbox
  • Bug 560193 - Allow deleting a saved search under news account
  • Bug 1503734 - Remove needless constants for nsMsgFolderFlags
  • Bug 473642 - the compact button should be disabled for saved searches
  • Bug 496543 - Marks folders as read only marks first folder
Depends on: 1600955

[Approval Request Comment]
Regression caused by (bug #): n/a
User impact if declined: no context menu when multiple items selected
Testing completed (on m-c, etc.): 2.53.x
Risk to taking this patch (and alternatives if risky): None
String changes made by this patch: Added 3 strings

Attachment #9183676 - Flags: review?(frgrahl)
Attachment #9183676 - Flags: approval-comm-release?
Attachment #9183676 - Flags: approval-comm-esr60?

Changes since last patch:

  • Moved newsgroup logic into for loop in deleteFolder so that it works properly.
  • Fixed FolderPaneController's isCommandEnabled to allow multiple folder selection.
  • Port some extra changes to take account of virtual folders in newsgroups
Attachment #9183676 - Attachment is obsolete: true
Attachment #9183676 - Flags: review?(frgrahl)
Attachment #9183676 - Flags: approval-comm-release?
Attachment #9183676 - Flags: approval-comm-esr60?
Attachment #9187880 - Flags: review?(frgrahl)
Attachment #9187880 - Flags: approval-comm-release?
Attachment #9187880 - Flags: approval-comm-esr60?

Changes since last patch:

  • Backport parts of a couple more patches
Attachment #9187880 - Attachment is obsolete: true
Attachment #9187880 - Flags: review?(frgrahl)
Attachment #9187880 - Flags: approval-comm-release?
Attachment #9187880 - Flags: approval-comm-esr60?
Attachment #9187914 - Flags: review?(frgrahl)
Attachment #9187914 - Flags: approval-comm-release?
Attachment #9187914 - Flags: approval-comm-esr60?

Comment on attachment 9187914 [details] [diff] [review]
Context menu for multiple selection patch v1.2

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.

Attachment #9187914 - Flags: review?(frgrahl)
Attachment #9187914 - Flags: feedback+
Attachment #9187914 - Flags: approval-comm-release?
Attachment #9187914 - Flags: approval-comm-esr60?

Changes since last patch:

  • Fixed disabling of Compact in file menu
  • Changed folder.flags to folder.getFlag in the new code
  • Added plural for compact on context menu
  • Addressed most of the other comments
Attachment #9187914 - Attachment is obsolete: true
Attachment #9190154 - Flags: review?(frgrahl)
Attachment #9190154 - Flags: approval-comm-release?
Attachment #9190154 - Flags: approval-comm-esr60?

Changes since last patch:

  • Disable compact in File menu for virtual folders.
Attachment #9190154 - Attachment is obsolete: true
Attachment #9190154 - Flags: review?(frgrahl)
Attachment #9190154 - Flags: approval-comm-release?
Attachment #9190154 - Flags: approval-comm-esr60?
Attachment #9190168 - Flags: review?(frgrahl)
Attachment #9190168 - Flags: approval-comm-release?
Attachment #9190168 - Flags: approval-comm-esr60?

Changes since last patch:

  • Do folder context menu separators the same way TB does.
Attachment #9190168 - Attachment is obsolete: true
Attachment #9190168 - Flags: review?(frgrahl)
Attachment #9190168 - Flags: approval-comm-release?
Attachment #9190168 - Flags: approval-comm-esr60?
Attachment #9190177 - Flags: review?(frgrahl)
Attachment #9190177 - Flags: approval-comm-release?
Attachment #9190177 - Flags: approval-comm-esr60?

Changes since last patch:

  • Properly fix separators in the folder pane context menu.
Attachment #9190177 - Attachment is obsolete: true
Attachment #9190177 - Flags: review?(frgrahl)
Attachment #9190177 - Flags: approval-comm-release?
Attachment #9190177 - Flags: approval-comm-esr60?
Attachment #9190189 - Flags: review?(frgrahl)
Attachment #9190189 - Flags: approval-comm-release?
Attachment #9190189 - Flags: approval-comm-esr60?
Blocks: 1679883

Comment on attachment 9190189 [details] [diff] [review]
Context menu for multiple selection patch v1.6

LGTM

Attachment #9190189 - Flags: review?(frgrahl)
Attachment #9190189 - Flags: review+
Attachment #9190189 - Flags: approval-comm-release?
Attachment #9190189 - Flags: approval-comm-release+
Attachment #9190189 - Flags: approval-comm-esr60?
Attachment #9190189 - Flags: approval-comm-esr60+
Depends on: 1680928

mailnews/ part is now in Bug 1680928, carrying forward r/a+

Attachment #9190189 - Attachment is obsolete: true
Attachment #9191508 - Flags: review+
Attachment #9191508 - Flags: approval-comm-release+
Attachment #9191508 - Flags: approval-comm-esr60+

Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/b944d175bb86
Port |Bug 296655 - selection of multiple mailboxes/folders in the folders pane is possible but not usable| to SeaMonkey. r=frg

Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Whiteboard: SM2.53.6
Target Milestone: --- → seamonkey 2.82

https://gitlab.com/seamonkey-project/seamonkey-2.53-comm/-/commit/5efb9c1e4f3f9d3b76a7fe19e7ba111d05e21e8a
Port |Bug 296655 - selection of multiple mailboxes/folders in the folders pane is possible but not usable| to SeaMonkey. r=frg a=frg

Does the fix for this also fix bug 50767 (or at least make it obsolete)? Looks like that's an old Mozilla Suite bug, referenced in Thunderbird bug 296655 comment 3.

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