Closed
Bug 473642
Opened 16 years ago
Closed 15 years ago
the compact button should be disabled for saved searches
Categories
(Thunderbird :: Mail Window Front End, defect)
Thunderbird
Mail Window Front End
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 3.0b3
People
(Reporter: mkmelin, Assigned: mkmelin)
References
(Depends on 1 open bug)
Details
Attachments
(1 file, 2 obsolete files)
7.90 KB,
patch
|
jminta
:
review+
|
Details | Diff | Splinter Review |
The compact button should be disabled for saved searches, as it does nothing for them.
Assignee | ||
Comment 1•16 years ago
|
||
This fixes it for saved searches, and also makes sure the compact button is disabled after startup (when a server node is selected) - I could reproduce that most of the times, but not always. Anyway, this is what the code should do as cmd_compactFolder is "compact all folders".
Attachment #361254 -
Flags: review?(jminta)
Assignee | ||
Updated•16 years ago
|
Status: NEW → ASSIGNED
Summary: the compact button should be disabled for saved searches → the compact button should be disabled for saved searches (and on startup when account node selected)
Target Milestone: --- → Thunderbird 3.0b2
Assignee | ||
Comment 2•16 years ago
|
||
Comment on attachment 361254 [details] [diff] [review]
proposed fix
Gah...
Attachment #361254 -
Attachment is obsolete: true
Attachment #361254 -
Flags: review?(jminta)
Assignee | ||
Comment 3•16 years ago
|
||
Maybe I can get it right this time:)
Attachment #361296 -
Flags: review?(jminta)
Assignee | ||
Comment 4•16 years ago
|
||
jminta: r ping
Target Milestone: Thunderbird 3.0b2 → Thunderbird 3.0b3
Comment 5•16 years ago
|
||
Comment on attachment 361296 [details] [diff] [review]
proposed fix, v2
This seems like a strange way to fix this. Right now isCommandEnabled* calls the selected folder's isCommandEnabled function. It seems to me that saved-search folders ought to return false there, which would cover this, no?
*Here I'm referring to the call to the terribly named function at the end of IsFolderCompactionEnabled, not the function-method of an nsICommandHandler.
Assignee | ||
Comment 6•16 years ago
|
||
The reason that's not enough is the startup case, where the defaultController will be used... and that needs to do all the same checks as the FolderPaneController.
Assignee | ||
Comment 7•15 years ago
|
||
Actually. I don't know what the startup thing was about anymore - i don't see it now.
Summary: the compact button should be disabled for saved searches (and on startup when account node selected) → the compact button should be disabled for saved searches
Assignee | ||
Comment 8•15 years ago
|
||
o button_compact is used both when the folder controller handles stuff and when the default handler does; and in the same way. Therefore i removed the handling from the folder controller, letting the default controller take all handling of it.
o function isCommandEnabled(cmd) was only used in very few places, so i'm removein that - didn't play nice with multi-select anyways.
o the compact was wrongly disabled for news - bug 460819.
Attachment #361296 -
Attachment is obsolete: true
Attachment #377221 -
Flags: review?(jminta)
Attachment #361296 -
Flags: review?(jminta)
Comment 9•15 years ago
|
||
Comment on attachment 377221 [details] [diff] [review]
proposed fix, v3
+ return folders.length == 1 && folders[0].canRename &&
+ folders[0].isCommandEnabled("cmd_renameFolder");
+ (folder.server.type != "imap" || folder.server.canCompactFoldersOnServer) &&
+ folder.isCommandEnabled("button_compact");
I just had a long irc discussion with dmose about the absurdity of (1) splitting this logic between the front and back ends, with particular checks in each and (2) even worse, having a canRename attribute that doesn't tell you everything, but also requires you to check isCommandEnabled("cmd_renameFolder"). (The same applies for the compact stuff)
The tentative conclusion we came to was that nsIMsgFolder::isCommandEnabled should die. Whether its current logic should move to the front-end, or should move to the appropriate nsIMsgFolder attributes remains a trickier question. Please file a follow-up to remove the method and sort out the proper location for that logic.
r=jminta with that bug filed. And bonus points for removing functions from the global scope.
Attachment #377221 -
Flags: review?(jminta) → review+
Assignee | ||
Comment 10•15 years ago
|
||
Filed bug 493016.
changeset: 2637:bb38d33f5744
http://hg.mozilla.org/comm-central/rev/bb38d33f5744
->FIXED
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•