Closed
Bug 274628
Opened 20 years ago
Closed 16 years ago
Preference to close message window on delete instead of loading next message.
Categories
(Thunderbird :: Preferences, enhancement)
Thunderbird
Preferences
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 3.0b3
People
(Reporter: pkopacz, Assigned: sheppy)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 9 obsolete files)
(deleted),
patch
|
mkmelin
:
review+
|
Details | Diff | Splinter Review |
When deleting a message from a message window, the next message is automatically
loaded. A preference setting where the message window is closed upon delete
instead would be useful.
Comment 1•20 years ago
|
||
Here's another vote for this feature. I would find this much more intuitive
than the current behavior. I'm constantly closing windows after deletion, very
rarely do I want to read the message that is then displayed.
The majority of one's messages these days are suspect or trivial, and an option
to not have them open automatically is a priority. Why is it not confirmed?
The problem is real.
Updated•19 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 3•19 years ago
|
||
*** Bug 302241 has been marked as a duplicate of this bug. ***
Comment 4•19 years ago
|
||
The buttons! extension has a close-on-delete option, but it isn't nicely
integrated as it could be (for instance, undo delete undoes normal
keep-window-open-and-advance delete, but not delete-and-close-window). This
seems like such a dirt-simple option to build into thunderbird, instead of
requiring an extension.
Comment 5•19 years ago
|
||
Buttons! does work, but this should be a built-in feature for Thunderbird.
Especially for newbs like me to the whole Mozilla scene, it took two heads about
10 minutes to figure out how to make Buttons! do the close on delete.
This bug is annoying whenever you are sifting spam, which is often. You know
the next message is rubbish that you do not want opened, but Thunderbird opens
it anyway. It should not be too hard to fix. At the last message it already
knows how to close the message window and return to the folder, so the code is
there. We need a switch in preferences so the user can choose.
As of 1.5, the buttons! extension no longer works, so adding this feature to the client is even more important.
Comment 8•19 years ago
|
||
I spent hours trying to figure out how to do this, only eventually to give up in despair. I can't think of another email client I've used that doesn't have this functionality. I find it annoying enough to consider not using Thunderbird.
Comment 9•19 years ago
|
||
There is an extention called "buttons!" that gives you a delete button that does this, but this quite a mess - it should be implemented anyhow.
Comment 10•19 years ago
|
||
Re: Comment #9,
Buttons! only works with 1.0x, so it's not really an acceptable solution.
Comment 11•19 years ago
|
||
I'm using buttons! with Thunderbird 1.5, I originally installed it on 1.0x and then upgraded without a hitch.
Comment 12•19 years ago
|
||
Here's a one-liner that adds this feature. It uses a disused Netscape preference, mail.close_message_window.on_delete, which controlled this behavior in Communicator at some point in this past. This preference is still in Thunderbird's default prefs (in mozilla/mailnews/mailnews.js), and defaults to true. The patch was created against the 1.5 tree, but is straightforwardly adaptable to newer trees as well.
Comment 13•19 years ago
|
||
Comment on attachment 216478 [details] [diff] [review]
a one-liner patch
I think this is reasonable, but I'd rename the pref to replace the window.on_delete with window_on_delete - there's no reason for there to be a '.' there.
Comment 14•19 years ago
|
||
I have verified that this patch works great under TB 1.5 & Windows 2k.
Comment 15•19 years ago
|
||
Given that this will probably take forever to make it into a release version, I have made a patched version of messenger.jar at http://bork.hampshire.edu/~alan/code/ which can be used with 1.5 (any OS).
Comment 16•19 years ago
|
||
(In reply to comment #13)
> (From update of attachment 216478 [details] [diff] [review] [edit])
> I think this is reasonable, but I'd rename the pref to replace the
> window.on_delete with window_on_delete - there's no reason for there to be a
> '.' there.
>
I didn't pick the name (as I said, it's an old Netscape preference), but the dot is there because there is another preference mail.close_message_window.on_file, so the naming scheme is <component>.<action>.<trigger-event>, which is pretty standard.
Comment 17•19 years ago
|
||
Here's a new version of my patch, though now it's much more than a one-liner, I think it's way more useful. In addition to closing the standalone window on delete, it tells the thread pane to select the message that the standalone window would show if this preference was turned off. This parallels the behavior of filing a message from the standalone window, as well as mimicing the behavior of other popular mail programs, such as Apple's Mail.app.
Attachment #216478 -
Attachment is obsolete: true
Comment 18•19 years ago
|
||
>so the naming scheme is
> <component>.<action>.<trigger-event>, which is pretty standard.
ah, ok, I didn't know that.
Updated•18 years ago
|
QA Contact: preferences
Comment 19•18 years ago
|
||
How do we get this into a stable release? It seems like a no-brainier to add this preference, at the very least, as an advanced option.
Comment 20•18 years ago
|
||
You get it into a stable release by first realizing that hidden prefs are actually more risky and expensive than visible prefs, because you then have to support behavior that nobody expects or sees, and thus everyone is more likely to break it, and then if you still think it shouldn't have visible UI, either take it over or help Jim learn to check at least that one file out from CVS, to create a |cvs diff -up8| patch, which will among other things automatically be in the right direction (the current patch says to remove the lines which it actually wants to add), and then ask one of the module owners or peers for review, by setting the review flag for the patch to "?" and pasting in his email.
Comment 21•18 years ago
|
||
This version has been updated to apply cleanly to the current development trunk, and also applies fine to Thunderbird 2.0.0. (I wish someone had pointed out long ago that I'd generated a backwards patch!).
Comment 22•18 years ago
|
||
(In reply to comment #20)
I'm agnostic on the question of whether there should be UI for this preference. It seems to me that it's not worth the effort because I believe that that this feature has a very small (if vocal) audience.
As for pushing for a review, someone else will have to take that job up. I actually don't think my patch is ultimately the right solution because it's essentially just copying equivalent functionality from msgMail3PaneWindow.js. The messageWindow.js and msgMail3PaneWindow.js naturally have a lot of what should be identical functionality, but the implementations are (imperfectly) duplicated when they should be shared. I don't have time to take this task on at the moment. My patch is, however, in keeping with the rest of the message window code, so if someone wants to push for review, go for it.
I also think perhaps the preference name should be changed to mail.close_message_window.on_delete_or_file or similar since the patch will close on either event right now. This is because there is only one event that covers both of these cases, DeleteOrMoveMsgCompleted, so there is currently no way to separate those two cases. I don't see a reason anyone would want to separate these two cases with regard to automatically closing the message window - the preference name is just a bit imprecise.
Comment 23•18 years ago
|
||
The old Netscape mail app had this feature, and was listed in the preference UI. I don't know what the argument was for removing it in Thunderbird. Most of the other mail readers that I've used have this option, so I'm not so sure it's such a niche option. There is a Thunderbird extension which is reasonably popular (Buttons) which adds this option, which is perhaps the reason why we don't hear about the need for it on bugzilla so much. The downside to the extension is that it's not always compatible with the latest release of Thunderbird.
Comment 25•17 years ago
|
||
(In reply to comment #24)
> *** Bug 420799 has been marked as a duplicate of this bug. ***
>
Okay, but what does it take to get this functionality flaw addressed? This has been open for over 2 years now.
Comment 26•17 years ago
|
||
Well, comment 20 had three parts, of which only one was addressed. Luckily, now we get to stick clarkbw with nasty decisions about which is the one true behavior, and what should or shouldn't have exposed pref UI :)
Comment 27•17 years ago
|
||
(In reply to comment #23)
>The old Netscape mail app had this feature, and was listed in the preference
>UI. I don't know what the argument was for removing it in Thunderbird.
Remember, Thunderbird was rewritten from scratch, so it was simply never added. (But it would be interesting to know what preference name Netscape used.)
Comment 28•17 years ago
|
||
It clearly should be an exposed pref. This is a standard feature in most mail readers, and a make or break feature for some people, myself included.
As for the other issues, none of us are close enough to the development team to know what to do next. Can somebody help us out here?
Comment 29•17 years ago
|
||
Well, I for one disagree this is something that should have UI.
Jim Alexander: are you planning on updating the patch? The attached version does not apply to current trunk. The hidden pref should also be added to all-thunderbird.js
Assignee: mscott → nobody
Comment 31•17 years ago
|
||
First time poster here, but I honestly someone does make a permanent fix for this, where after deleting a message, the next message does not come up.
Thanks,
Dave C.
Comment 32•16 years ago
|
||
Yikes... I just wasted a few hours trying to figure out why this wasn't working. :( It didn't seem possible that it simply wouldn't be present, as it's such a commonly-available feature... I don't think that I've *ever* encountered another GUI email client which didn't have this behavior, at least as an option.
Hard to believe this has languished for nearly 4 years now.
Comment 33•16 years ago
|
||
(In reply to comment #16)
> I didn't pick the name (as I said, it's an old Netscape preference), but the
> dot is there because there is another preference
> mail.close_message_window.on_file, so the naming scheme is
> <component>.<action>.<trigger-event>, which is pretty standard.
No, we don't have such a pref.
Comment 34•16 years ago
|
||
You might want to look at the date on that comment before making that claim. The preference was there at the time, but were removed starting with CVS version
<A HREF="http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=mailnews.js&branch=&root=/cvsroot&subdir=mozilla/mailnews&command=DIFF_FRAMESET&rev1=3.297&rev2=3.298">3.298</A>
Comment 36•16 years ago
|
||
Is there a reason why the default isn't just to close the window when the message is deleted? Opening another message in the same window seems kind of odd.
Comment 37•16 years ago
|
||
Happy 2009! So my bug has been redirected here. And I've read from start to
here and still am unsure what I can do to get the functionality in TB. Do I
apply Jim Alexander's patch that in his May 2007 note? Or should I start
MUTTering instead?
Comment 38•16 years ago
|
||
(In reply to comment #36)
> Is there a reason why the default isn't just to close the window when the
> message is deleted?
Yes, that way you can read through your mails in the standalone window.
Updated•16 years ago
|
Flags: wanted-thunderbird3?
Comment 39•16 years ago
|
||
Since the TB developers seem to view this as a feature request rather than a bug, is there some other avenue that should be pursued? I find it hard to believe that, given TB has gone through 2.0 and 3.0 since this request was put in that NOBODY officially with TB has had time to look at what seems to be a simple request. I don't believe this is something that can be accomplished by end users themselves, unless the official TB developers want end users making UI decisions unilaterally.
Comment 40•16 years ago
|
||
I now use Outlook 2003 at work and it as the feature of closing the windows after deleting the message, so this bug should be linked to Bug 423488 - Outlook Express/Outlook parity bugs
I test it into TB3beta1 and the problem is still here, even in tab...
Comment 41•16 years ago
|
||
I feel this is more of a UI consistency issue than a feature request. If you search your mailbox and then open a message in a new window from there you get the desired results of the message window closing when you delete the message.
Comment 42•16 years ago
|
||
What version of TB are you using? If I open email in its own window and delete that message it gets replaced by the next one in my queue. I switched to Evolution couple months ago just to try it out and I get the same sort of behavior. Only MSOutlook acts as I expect - maybe Bill got something right here.
Comment 43•16 years ago
|
||
I think that is Evolution emulating TB...every previous GUI email reader I can think of that I've tried (Eudora and Netscape amongst them) have had close on delete, most of them as a mandatory default.
Assignee | ||
Comment 44•16 years ago
|
||
For the record, this patch appears to work fine on comm-central, at least after a few minutes of trying it out.
Comment 45•16 years ago
|
||
Bryan, any input on this? Do we want UI for such a pref, if we take the patch?
Comment 46•16 years ago
|
||
I think we could take this patch. We've made space in the Advanced preferences for a Reading & Display tab; I think we could use a little of that space for this.
My UI diff would look something like this:
Open Messages In:
....
[ ] Close message window on delete
-- Display -----
Assignee | ||
Comment 47•16 years ago
|
||
It looks like this only works if you opened the message from a regular folder; if you opened the message from a saved search, clicking the delete button does nothing (doesn't even delete the message).
Comment 48•16 years ago
|
||
So, anybody up for finalizing the patch (adding the pref UI and checking whatever needs to be done about comment 47)?
Keywords: helpwanted
Hardware: x86 → All
Assignee | ||
Comment 49•16 years ago
|
||
Comment 47, it turns out, affects all the buttons in the toolbar, not just delete, so it's an unrelated issue (see bug 395495).
Assignee | ||
Comment 50•16 years ago
|
||
I'm taking a whack at turning this into a real patch including the UI. Will be my first time trying to do anything with the UI patch-wise, so this'll be fun!
Assignee | ||
Comment 51•16 years ago
|
||
Here's a patch that includes the UI update, adds the pref to the defaults file, and handles the close on delete behavior. I'm not super experienced with this process so I'm not setting any flags on this; would appreciate it if someone would try this out and do so if it looks good.
Attachment #217293 -
Attachment is obsolete: true
Attachment #264163 -
Attachment is obsolete: true
Comment 52•16 years ago
|
||
Comment on attachment 367106 [details] [diff] [review]
Close on delete patch for comm-central, with UI updated
You shouldn't use X as an accesskey, as it's not included in any of the words.
Since you're not implementing it for seamonkey, the pref should not be in mailnews.js but in all-thunderbird.js
However, this patch doesn't work when you use the imap mark as deleted model. (The window doesn't close.)
Assignee | ||
Comment 53•16 years ago
|
||
I suspect that the problem with not closing is related to bug 395495.
Assignee | ||
Comment 54•16 years ago
|
||
Changed the access key and moved the pref to all-thunderbird.js
Attachment #367106 -
Attachment is obsolete: true
Comment 55•16 years ago
|
||
(In reply to comment #53)
> I suspect that the problem with not closing is related to bug 395495.
No, looks like it's due to
if ((folder.URI == gCurrentFolderUri) && gCurrentMessageIsDeleted)
check earlier in HandleDeleteOrMoveMsgCompleted.
Updated•16 years ago
|
Assignee: nobody → eshepherd
Keywords: helpwanted
Assignee | ||
Comment 56•16 years ago
|
||
Looks like the HandleDeleteOrMoveMsgCompleted method doesn't get called if the message is only marked as deleted and not actually removed. I'm looking into adding the required code where it belongs to handle closing the window when the message is simply marked for deletion. I hope to have a new patch soon.
Assignee | ||
Comment 57•16 years ago
|
||
Looking at this further, I'm not convinced the message window should close in "mark as deleted" mode. The code in nsImapMailFolder.cpp, starting at line 2149, is specifically only sending the notification of a message being moved or deleted if you're not using the mark as deleted model. The fact that the toolbar icon changes to "Undelete" in this situation also hints that perhaps in the mark as deleted mode, the window shouldn't close.
Opinions?
Assignee | ||
Comment 58•16 years ago
|
||
Another reason to perhaps not close the window on delete in "mark as deleted" mode: deleting the message in this mode doesn't seem to automatically advance to the next message. Any thoughts?
Comment 59•16 years ago
|
||
Sounds like a behavior bug to me. I'm sure fixing that now will cause a stir with people who have been experiencing it for a while now and expect the alternate behavior.
If we choose a behavior that's based on I think we will have a difficult time explaining to users the different behavior of the window closing depending on the delete mode the IMAP account is using. Closing it would conflict with the fact, you bring up, that it's never loaded a new message before; but then closing the window seems like what users would end up doing in that situation anyway.
Assignee | ||
Comment 60•16 years ago
|
||
I'd be inclined to say that the patch I've submitted here (or a variation on it if any other issues come up) resolves this particular bug, and that a separate bug should be filed to consider changing the behavior when deleting a message in mark-as-deleted mode.
Assignee | ||
Comment 61•16 years ago
|
||
Who should review this patch?
Assignee | ||
Updated•16 years ago
|
Attachment #367222 -
Flags: review?(mkmelin+mozilla)
Comment 62•16 years ago
|
||
Comment on attachment 367222 [details] [diff] [review]
Revised close-on-delete patch
>+ if (pref.getBoolPref("mail.close_message_window.on_delete")){
>+ // tell the main window to select the next message since we
>+ // won't be viewing it automatically in the standalone
window
Please capitalize and end the sentence with a dot.
>+ var mainWindow = window.opener;
>+ var mainDoc = mainWindow.document;
No need for the temporary mainWindow varibable.
>+ var treeView = mainDoc.getElementById("threadTree").view;
>+ var treeSelection;
>+ if (treeView)
>+ treeSelection = treeView.selection;
>+
>+ gDBView.suppressCommandUpdating = true;
>+
>+ if (gNextMessageViewIndexAfterDelete >= 0)
>+ treeSelection.select(gNextMessageViewIndexAfterDelete);
>+
>+ if (treeView)
>+ treeView.selectionChanged();
The above is a bit odd. Wither we have a treeView or we will have an undefined treeSelection anyway. The whole thing should be inside the if block - if it's needed at all...
Also remove the trailing spaces there.
>+ window.opener.EnsureRowInThreadTreeIsVisible(gNextMessageViewIndexAfterDelete);
>+ gDBView.suppressCommandUpdating = false;
>+ window.close();
>+ } else if (nextMstKey != nsMsgKey_None) {
Please put the "else if..." on a new line.
>+<!ENTITY closeMsgWindowOnDelete.label "Close message window on delete">
>+<!ENTITY closeMsgWindowOnDelete.accesskey "c">
Accesskeys are actually case sensitive (for performance, both upper and lower "works" though.
Assignee | ||
Comment 63•16 years ago
|
||
New patch to address Magnus's comments in c#62.
Attachment #368990 -
Flags: review?(mkmelin+mozilla)
Assignee | ||
Updated•16 years ago
|
Attachment #367222 -
Attachment is obsolete: true
Attachment #367222 -
Flags: review?(mkmelin+mozilla)
Comment 64•16 years ago
|
||
Unbitrotted the fix. I've played with it some - don't think the if (treeView) was necessary + when the pref to close is set I don't think it should advance with imap-mark-as-deleted.
This part seems to work well now with this patch. However, from the search results the behavior is quite weird: the next selected msg gets all wrong it seems... (only when the close-pref is set). Can you investigate that?
Attachment #368990 -
Attachment is obsolete: true
Attachment #368990 -
Flags: review?(mkmelin+mozilla)
Assignee | ||
Comment 65•16 years ago
|
||
There are many things that don't work right with search results. Is there something specifically different with the threadTree when it's showing search results? I can't figure that part out, but there must be something...
Assignee | ||
Comment 66•16 years ago
|
||
OK, HandleDeleteOrMoveMsgCompleted is never getting called when you hit the delete button on a message opened from search results... investigation continues.
Assignee | ||
Comment 67•16 years ago
|
||
Looks like even though the folderListener is being set up by calling AddFolderListener, the listener never gets called. Not sure why not. Anyone have a guess?
Assignee | ||
Comment 68•16 years ago
|
||
OK, I've got this figured out now. Testing a patch that fixes the delete, mark, and flag buttons to all work. Seems to be working so far; I expect to attach a patch sometime Saturday.
Assignee | ||
Comment 69•16 years ago
|
||
OK, my patch so far is only working for the delete button; investigating why it's not helping for the others.
Assignee | ||
Comment 70•16 years ago
|
||
I believe this fixes the problem, both for the delete button and the mark button, and handles exceptions a little better. I'd like someone with a better internal knowledge of Thunderbird to give it a look and see what they think of the patch; I wonder if it might be done better, but am unsure how.
Attachment #373487 -
Attachment is obsolete: true
Comment 71•16 years ago
|
||
What exception are you catching?
Assignee | ||
Comment 72•16 years ago
|
||
That's a good question; I based my patch on other code, which doesn't actually check what exception is thrown that I can see.
Assignee | ||
Comment 73•16 years ago
|
||
Should I change the patch to only catch the invalid pointer exception that's responsible for the problem here in particular? Not sure whether that matters or not in this context, but certainly I can do that.
Assignee | ||
Comment 74•16 years ago
|
||
This version of the patch now actually checks the exception being thrown and only handles NS_ERROR_INVALID_POINTER, which is the one that means the message is standalone in this context. It does this for the delete, flagged, and mark buttons.
Attachment #374789 -
Attachment is obsolete: true
Attachment #374951 -
Flags: review?(mkmelin+mozilla)
Comment 75•16 years ago
|
||
(In reply to comment #66)
> OK, HandleDeleteOrMoveMsgCompleted is never getting called when you hit the
> delete button on a message opened from search results...
I don't know what you were seeing but HandleDeleteOrMoveMsgCompleted gets called alright over here. I'm also unable to get those exceptions you catch...
Have to investigate more tomorrow.
Assignee | ||
Comment 76•16 years ago
|
||
Conditions have changed in recent builds since other bugs were fixed; I'm working on tweaking this patch; much of it may no longer be needed. Can't get a build to go tonight, looks like I checked out broken code. Going to try again tomorrow.
Comment 77•16 years ago
|
||
Comment on attachment 374951 [details] [diff] [review]
More intelligent patch
Cancelling this in anticipation of the new patch.
Attachment #374951 -
Attachment is obsolete: true
Attachment #374951 -
Flags: review?(mkmelin+mozilla)
Assignee | ||
Comment 78•16 years ago
|
||
This patch takes out a bunch of stuff that's not needed now that other stuff elsewhere has been fixed (the previous patch also fixed another problem, which no longer needs to be done).
Attachment #376838 -
Flags: review?(mkmelin+mozilla)
Updated•16 years ago
|
Attachment #376838 -
Flags: review?(mkmelin+mozilla) → review+
Comment 79•16 years ago
|
||
Comment on attachment 376838 [details] [diff] [review]
Unbitrotted patch
Looks good, thx for hanging in there! r=mkmelin
Comment 80•16 years ago
|
||
I changed a couple of style nits and checked this in.
changeset: 2664:1cdf3c60c94c
http://hg.mozilla.org/comm-central/rev/1cdf3c60c94c
->FIXED
Status: NEW → RESOLVED
Closed: 16 years ago
Flags: wanted-thunderbird3? → wanted-thunderbird3+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0b3
Comment 81•15 years ago
|
||
This new preference no longer works starting with the Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1pre) Gecko/20090613 Lightning/1.0pre Shredder/3.0b3pre build.
Comment 82•15 years ago
|
||
(In reply to comment #81)
> This new preference no longer works starting with the Mozilla/5.0 (Windows; U;
> Windows NT 6.0; en-US; rv:1.9.1pre) Gecko/20090613 Lightning/1.0pre
> Shredder/3.0b3pre build.
Please file a new bug and mark it as blocking bug 497199. It is most likely bug 474701 that has broken this, and therefore we should deal wqith it in a new bug.
Comment 83•15 years ago
|
||
Done. File as bug 498141.
You need to log in
before you can comment on or make changes to this bug.
Description
•