Closed
Bug 80574
Opened 23 years ago
Closed 15 years ago
Standalone: [pref] Delete should close the standalone window
Categories
(SeaMonkey :: MailNews: Message Display, enhancement)
SeaMonkey
MailNews: Message Display
Tracking
(Not tracked)
RESOLVED
FIXED
seamonkey2.0b1
People
(Reporter: hwaara, Assigned: InvisibleSmiley)
References
()
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
mnyromyr
:
review+
neil
:
superreview+
|
Details | Diff | Splinter Review |
Quoting from the spec (URL is attached):
``The "Delete" toolbar button deletes (moves to Trash) the message and closes
the standalone window.''
Currently, the next message is viewed in the standalone window if Delete is
performed.
Comment 3•23 years ago
|
||
I think the spec hasn't been updated. Before the recent performance
improvements we didn't have the ability to do navigation in the standalone
window so we made delete and move close the window. Now that's been
implemented. So, I'm going to reassign this to jglick to update the spec.
Assignee: sspitzer → jglick
Reporter | ||
Comment 4•23 years ago
|
||
No, why should we just update the spec to reflect the current behaviour?
The reason I filed this bug in the first place was that this behaviour is wrong
and illogical.
Jglick, comments?
Comment 5•23 years ago
|
||
4.x does not close the window but appears to display the next message message. I
like this functionality because I know there are many people who view all their
messages in a stand alone window. After deleting a message it's convenient to
see the next message. If they want to close the window then they select
File|Close or the "x" box.
This is one I actually think deserves a preference. Some users like to see the
next message when they delete a message and other users like to see the message
close when they delete it. (There might be an older bug with a similar
discussion...) Displaying the next message would be the default.
What do folks think of having this as a pref?
I wouldn't mind a pref (default==4xp). but if we don't create a pref I want the
4xp (which would mean this=>WONTFIX).
Severity: normal → enhancement
Summary: Standalone: Delete should close the standalone window → Standalone: [pref] Delete should close the standalone window
Comment 9•23 years ago
|
||
How about in Preferences/Mail and Newsgroups/General settings...
[*] Read next message after delete
I would like to see this as a pref. I prefer messages to close so that I don't
accidentaly open a message I didn't really want to open (spam). But I can see
where people with large volumes of mail would want to proceed to the next
message after deleting or replying to a message.
Comment 10•23 years ago
|
||
Just another thought, I was visiting an office and someone pointed out that
they like to read their mail from bottom to top, so the pref should allow for
that.
After deleting a message go to [mail folder|next message|previous message|v]
But that can be left as a future enhancement.
Comment 11•23 years ago
|
||
See also bug 90539, delete button on keyboard does not work when navigating
from mail window.
Comment 12•23 years ago
|
||
If you are on the last message (there is no "next" message) and you press the
delete button, the message is deleted, but the window remains open and continues
to display the now-deleted message. This behavior is pretty confusing.
Comment 13•23 years ago
|
||
Given the amount of spam that I and no doubt many others must deal with today I
very much miss the 4.x behavior that allows me to select and delete messages in
the preview pane without advancing to the next message.
The problem with mozilla 0.9.7 is that MailNews always advances to the next
unread (or last) message when one or more messages are deleted. This usually
results in having to view some overcomplicated html spam message that then
occupies mozilla while it loads up for a few seconds before I can do anything else.
For example:
In 3 pane mode.
1) Select messages to de deleted including the last (or most current message) in
the preview pane.
2) Hit delete key - messages are marked for deletion
3) Mailnews then advances and displays the last message in the preview pane even
though it has been marked for deletion
4) Compact folder
5) I still see the last deleted message in the preview pane (this is a bug)
which is usually spam anyway.
6) Manually select another message in list to clear preview pane of unwanted
spam message that has already been deleted.
reproducable 100%
Solaris 8
Mozilla 0.9.7
I much prefer the 4.x behavior which does not advance to *any* message after the
delete key is pressed. When the currently displayed message is actually removed
then n4.x either advances to the next message after the deleted message or if
the last message was deleted then it displays whatever the last message is (read
or unread) left in the list.
Call this a feature if you want but in this day and age of constant spam
anything that can help deal with it would be greatly appreciated.
Comment 14•23 years ago
|
||
Adding link to proposed pref panel. Reassigning back to default owner.
Assignee: jglick → sspitzer
QA Contact: sheelar → esther
Comment 15•22 years ago
|
||
*** Bug 153837 has been marked as a duplicate of this bug. ***
Comment 16•22 years ago
|
||
*** Bug 194112 has been marked as a duplicate of this bug. ***
Updated•20 years ago
|
Product: Browser → Seamonkey
Updated•20 years ago
|
Assignee: sspitzer → mail
Comment 17•19 years ago
|
||
*** Bug 239306 has been marked as a duplicate of this bug. ***
Updated•16 years ago
|
Assignee: mail → nobody
QA Contact: esther → message-display
Assignee | ||
Comment 18•15 years ago
|
||
Port from TB bug 274628. I had to remove the separators in the pref pane to make it fit (else at least on Windows the bottom hbox is cut off).
Assignee: nobody → jh
Status: NEW → ASSIGNED
Attachment #380048 -
Flags: superreview?(neil)
Attachment #380048 -
Flags: review?(mnyromyr)
Comment 19•15 years ago
|
||
Comment on attachment 380048 [details] [diff] [review]
proposed patch
>+ // Tell the main window to select the next message since we
>+ // won't be viewing it automatically in the standalone window.
How do you know that the main window was showing the deleted message? Instead of this change I think it might be worth a separate patch/bug for what to do when deleting a message that's being shown in another window.
>- <separator class="thin"/>
Maybe we should put this pref in another pane that has a bit more room?
Assignee | ||
Comment 20•15 years ago
|
||
(In reply to comment #19)
> (From update of attachment 380048 [details] [diff] [review])
> >+ // Tell the main window to select the next message since we
> >+ // won't be viewing it automatically in the standalone window.
> How do you know that the main window was showing the deleted message? Instead
> of this change I think it might be worth a separate patch/bug for what to do
> when deleting a message that's being shown in another window.
Agreed, removed from the alternate patch.
> >- <separator class="thin"/>
> Maybe we should put this pref in another pane that has a bit more room?
The only other place would be the main Mail & Newsgroups category but it fits so much better in Message Display, also because there's already "When opening messages..." there which refers to standalone message windows. Since I placed the new relating pref directly below it there's no real need for a separator between those, only possibly below the new checkbox line. But IMO that's not really that much of an issue either because a list of checkboxes pleases the eye.
In my alternate patch I've also changed the Font pref under "Plain text messages" from an ugly 2-items dropdown to a horizontal radiogroup and removed the separator below it, allowing me to bring back the separator above "Automatically mark messages as read". I think the pref pane looks better that way overall and hope you agree.
Attachment #380048 -
Attachment is obsolete: true
Attachment #380048 -
Flags: superreview?(neil)
Attachment #380048 -
Flags: review?(mnyromyr)
Attachment #380249 -
Flags: superreview?(neil)
Attachment #380249 -
Flags: review?(mnyromyr)
Comment 21•15 years ago
|
||
Comment on attachment 380249 [details] [diff] [review]
alternate patch
>+++ b/suite/locales/en-US/chrome/mailnews/pref/pref-viewing_messages.dtd
>+<!ENTITY closeMsgWindowOnDelete.label "Close message window on delete">
I'd prefer something like "Close message window when deleting the message".
>+<!ENTITY closeMsgWindowOnDelete.accesskey "l">
>\ No newline at end of file
And while you're at it, please fix this as well. ;-)
>+++ b/suite/mailnews/messageWindow.js
> function HandleDeleteOrMoveMsgCompleted(folder)
> {
>+ var folderResource = folder.QueryInterface(Components.interfaces.nsIRDFResource);
>+ if (!folderResource)
>+ return;
if (!(folderResource instanceof Components.interfaces.nsIRDFResource))
return;
> if ((folderResource.Value == gCurrentFolderUri))
Kill the double braces while you're at it.
>+++ b/suite/mailnews/prefs/pref-viewing_messages.xul
>+ <radiogroup id="mailFixedWidthMessages"
>+ orient="horizontal" class="indent"
>+ preference="mail.fixed_width_messages">
>+ <radio value="true" label="&fontFixedWidth.label;"
>+ accesskey="&fontFixedWidth.accesskey;"/>
>+ <radio value="false" label="&fontVarWidth.label;"
>+ accesskey="&fontVarWidth.accesskey;"/>
Either put all attributes onto one line or use one line per attribute and align them vertically; generally order 'id' before 'label' before 'accesskey' before stuff before command handlers.
r=me with that.
Attachment #380249 -
Flags: review?(mnyromyr) → review+
Comment 22•15 years ago
|
||
Comment on attachment 380249 [details] [diff] [review]
alternate patch
>+// close standalone message window when deleting the displayed message
>+pref("mail.close_message_window.on_delete", false);
[I would have thought this would have been more appropriate near the reuse message window pref, see below.]
>\ No newline at end of file
[As Mnyromyr mentioned.]
>- var folderResource = folder.QueryInterface(Components.interfaces.nsIRDFResource);
>- if (!folderResource)
>- return;
>+ var folderResource = folder.QueryInterface(Components.interfaces.nsIRDFResource);
>+ if (!folderResource)
>+ return;
>
> if ((folderResource.Value == gCurrentFolderUri))
If you're going to fiddle with this you might as well write (I think)
if (folder.URI == gCurrentFolderUri)
> preference="mailnews.reuse_message_window">
> <radio value="false" label="&newWindowRadio.label;"
> accesskey="&newWindowRadio.accesskey;" id="new"/>
> <radio value="true" label="&existingWindowRadio.label;"
> accesskey="&existingWindowRadio.accesskey;" id="existing"/>
> </radiogroup>
> </hbox>
>
>- <separator class="thin"/>
>-
>+ <checkbox id="closeMsgWindowOnDelete"
>+ label="&closeMsgWindowOnDelete.label;"
>+ accesskey="&closeMsgWindowOnDelete.accesskey;"
>+ preference="mail.close_message_window.on_delete"/>
I think this checkbox looks as if it has some sort of relationship to the previous preference (after all if you reuse the message window you are less likely to want it to close on delete) and it therefore might be worth adding class="indent" to it (and then also move the pref above).
>+ <radiogroup id="mailFixedWidthMessages"
>+ orient="horizontal" class="indent"
On the other hand this class="indent" is wrong and should be removed. sr=me with that fixed.
Attachment #380249 -
Flags: superreview?(neil) → superreview+
Assignee | ||
Comment 23•15 years ago
|
||
(In reply to comment #21)
> >+++ b/suite/mailnews/messageWindow.js
> > function HandleDeleteOrMoveMsgCompleted(folder)
> > {
> >+ var folderResource = folder.QueryInterface(Components.interfaces.nsIRDFResource);
> >+ if (!folderResource)
> >+ return;
>
> if (!(folderResource instanceof Components.interfaces.nsIRDFResource))
> return;
With that change, folderResource would be undefined. ;-) I followed Neil's suggestion here but of course using 'folder' (in two places) would work, too.
> >+++ b/suite/mailnews/prefs/pref-viewing_messages.xul
> >+ <radiogroup id="mailFixedWidthMessages"
> >+ orient="horizontal" class="indent"
> >+ preference="mail.fixed_width_messages">
> >+ <radio value="true" label="&fontFixedWidth.label;"
> >+ accesskey="&fontFixedWidth.accesskey;"/>
> >+ <radio value="false" label="&fontVarWidth.label;"
> >+ accesskey="&fontVarWidth.accesskey;"/>
>
> Either put all attributes onto one line or use one line per attribute and align
> them vertically; generally order 'id' before 'label' before 'accesskey' before
> stuff before command handlers.
OK but I kept 'value' in front (cf. reuseMessageWindow). Or do you count that as "stuff"?
(In reply to comment #22)
> > preference="mailnews.reuse_message_window">
> > <radio value="false" label="&newWindowRadio.label;"
> > accesskey="&newWindowRadio.accesskey;" id="new"/>
> > <radio value="true" label="&existingWindowRadio.label;"
> > accesskey="&existingWindowRadio.accesskey;" id="existing"/>
> > </radiogroup>
> > </hbox>
> >
> >- <separator class="thin"/>
> >-
> >+ <checkbox id="closeMsgWindowOnDelete"
> >+ label="&closeMsgWindowOnDelete.label;"
> >+ accesskey="&closeMsgWindowOnDelete.accesskey;"
> >+ preference="mail.close_message_window.on_delete"/>
> I think this checkbox looks as if it has some sort of relationship to the
> previous preference (after all if you reuse the message window you are less
> likely to want it to close on delete)
I was also thinking that.
> and it therefore might be worth adding
> class="indent" to it (and then also move the pref above).
The radio button and checkbox aren't perfectly aligned now but I guess we have to live with that.
Attachment #380249 -
Attachment is obsolete: true
Attachment #381719 -
Flags: superreview+
Attachment #381719 -
Flags: review?(mnyromyr)
Comment 24•15 years ago
|
||
Comment on attachment 381719 [details] [diff] [review]
patch v2, sr=neil
(In reply to comment #23)
> I followed Neil's suggestion here
Yeah, that's even better.
> OK but I kept 'value' in front (cf. reuseMessageWindow).
> Or do you count that as "stuff"?
I do, yes.
> > and it therefore might be worth adding
> > class="indent" to it (and then also move the pref above).
>
> The radio button and checkbox aren't perfectly aligned now but I guess we have
> to live with that.
No, we don't. ;-)
You need to put the class onto the first radio, not onto the outer hbox.
Attachment #381719 -
Flags: review?(mnyromyr) → review-
Comment 25•15 years ago
|
||
(In reply to comment #23)
> (In reply to comment #22)
> > > </radiogroup>
> > > </hbox>
> > >
> > >+ <checkbox id="closeMsgWindowOnDelete"
> > and it therefore might be worth adding
> > class="indent" to it (and then also move the pref above).
> The radio button and checkbox aren't perfectly aligned now but I guess we have
> to live with that.
About 9 years ago the box was added to indent the radiogroup, when in fact it wasn't necessary because class="indent" works fine on a radiogroup, although nobody noticed (including me, when I reviewed several subsequent patches).
However class="indent" is not good on a checkbox, because the checkbox has its own default margin which conflicts, and I should have realised that.
You could put the checkbox inside its own box. Or, you could change the radiogroup's box into a vbox and put the checkbox inside that. (You would want to remove the align if you did that though!)
Assignee | ||
Comment 26•15 years ago
|
||
Attachment #381719 -
Attachment is obsolete: true
Attachment #381785 -
Flags: superreview?(neil)
Attachment #381785 -
Flags: review?(mnyromyr)
Updated•15 years ago
|
Attachment #381785 -
Flags: superreview?(neil) → superreview+
Comment 27•15 years ago
|
||
Comment on attachment 381785 [details] [diff] [review]
patch v3 [Checkin: Comment 28]
[I wouldn't have gone to the trouble of rewrapping the existing radios]
Updated•15 years ago
|
Attachment #381785 -
Flags: review?(mnyromyr) → review+
Comment 28•15 years ago
|
||
Comment on attachment 381785 [details] [diff] [review]
patch v3 [Checkin: Comment 28]
Landed as <http://hg.mozilla.org/comm-central/rev/eaaa196d5af0> with the radios' value attribute made last, so that id is first.
Assignee | ||
Updated•15 years ago
|
Attachment #381785 -
Attachment description: patch v3 → patch v3 [Checkin: Comment 28]
Assignee | ||
Updated•15 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0b1
You need to log in
before you can comment on or make changes to this bug.
Description
•