implement mail "back" and "forward"

VERIFIED FIXED in Thunderbird2.0

Status

Thunderbird
Mail Window Front End
VERIFIED FIXED
17 years ago
10 years ago

People

(Reporter: (not reading, please use seth@sspitzer.org instead), Assigned: Bienvenu)

Tracking

({verified1.8.1, verified1.8.1.3})

Trunk
Thunderbird2.0
x86
All
verified1.8.1, verified1.8.1.3
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(7 attachments, 3 obsolete attachments)

there is now UI, it was just accelerator driven.

bienvenu was telling me how 4.x had this.  sounded useful.
future
Target Milestone: --- → Future

Comment 2

17 years ago
*** Bug 77099 has been marked as a duplicate of this bug. ***

Comment 3

17 years ago
Bug 77099 has recently been marked as a duplicate of this one. However, its
scope is slightly different (it requests a list of recently read messages), so,
to make sure no one forgets about these requests, I'll quote the relevant bits here:

Opened: 2001-04-22 10:00 by lasse@lama.no (Lasse Marøen)

Description:

Sometimes I have missed the possibility of navigating between messages in
mailnews based on the order which they were read. It would be great to have the
opportunity of going back to read messages without having to recreate sorting
method and scrolling up and down.

What I'd like is something like the History Sidebar, only for Mailnews. That way
one could easily find a message based on search criteria such as "I'm sure I
read this half an hour ago" or "someone said something about that in some
newsgroup I read yesterday".


------- Additional Comments From Alex Bishop 2001-04-22 10:37 -------

So basically you want a list of say, the ten last messages you read, a little
like the most recently accessed pages in Navigator's Go menu (but persistent
across sessions). That's a nice idea and I don't think it would be too hard to
implement it. Probably wouldn't make 1.0 though.


------- Additional Comments From Lasse Marøen 2001-04-22 12:30 -------

Ideally I would have all the navigation features of navigator in Mailnews - Back
and forward buttons, a go menu, and the History Sidebar. I would settle for the
latter though, I guess it's a feature that wouldn't be used enough to justify a
new menu or new buttons in the mail UI.
(Assignee)

Comment 4

16 years ago
I really miss this from 4.x - I'll take this bug and maybe find some time to do
it (back and forward, not the rest of the stuff described here)
Assignee: sspitzer → bienvenu

Comment 5

16 years ago
adding self to cc list

Updated

15 years ago
Blocks: 161374

Comment 6

14 years ago
Back is now implemented in Thunderbird. Any plans of copying that code to
Mozilla's Mail?
(Assignee)

Comment 7

14 years ago
afaik, back isn't implemented in thunderbird; previous unread is implemented.
They're not the same.

Comment 8

14 years ago
*** Bug 234841 has been marked as a duplicate of this bug. ***

Comment 9

14 years ago
*** Bug 163880 has been marked as a duplicate of this bug. ***

Comment 10

14 years ago
*** Bug 169947 has been marked as a duplicate of this bug. ***

Comment 11

13 years ago
Will this never be implemented? 3 years later, Thunderbird still does not have this.
Product: Browser → Seamonkey

Comment 12

12 years ago
There's an extension (now) named 'Mistory' that implements this for Mozilla, SM
and TB: <http://www.heise.de/ix/artikel/2005/09/154/mistory-1.0.tar.gz>, but
it's more of a proof of concept. (It's part of an article about XUL.)
(Assignee)

Comment 13

11 years ago
Created attachment 230842 [details] [diff] [review]
work in progress

work in progress. The back menu seems to be working - the forward menu isn't quite right yet. The other remaining thing I want to do is make back do the right thing when you read a message and then go to a different folder w/o selecting a message - back should go back to the message you were reading in the previous folder.
(Assignee)

Comment 14

11 years ago
Created attachment 231660 [details] [diff] [review]
more work in progress ...

this is quite a bit closer to being done - I still need to prune the global history at some point (maybe 100 messages?), and fix a problem where an empty entry gets into the dropdown menu.

I also need to add code to enable/disable forward and back correctly.
Attachment #230842 - Attachment is obsolete: true
(Assignee)

Comment 15

11 years ago
Created attachment 233310 [details] [diff] [review]
proposed fix

this is getting to where I'd like to get it reviewed and landed...
Attachment #233310 - Flags: superreview?(mscott)

Comment 16

11 years ago
Comment on attachment 233310 [details] [diff] [review]
proposed fix

woot!

you should be able to use the new icons which are now on the branch and trunk for forward / back. The last two icons in:

http://lxr.mozilla.org/seamonkey/source/mail/themes/qute/mail/icons/mail-toolbar.png
and
http://lxr.mozilla.org/seamonkey/source/mail/themes/qute/mail/icons/mail-toolbar-small.png

We don't have artwork for the mac yet. 

We should ditch the dumps statement:

+    dump("msg uri = " + msgUriStr.toString +  "\n");

Is this messenger object the right place to put this stuff or should it be part of the mail window? Browser page history is window specific I think...

can you do a quick leak test to make sure the messenger object still gets released properly now that it's a registered folder listener?
Attachment #233310 - Flags: superreview?(mscott) → superreview+

Comment 17

11 years ago
Comment on attachment 233310 [details] [diff] [review]
proposed fix

Earlier you have
>+  var folderUri = messenger.getFolderUriAtNavigatePos(historyIndex);
>+  var msgUri = messenger.getMsgUriAtNavigatePos(historyIndex);
>+  var folder = RDF.GetResource(folderUri).QueryInterface(Components.interfaces.nsIMsgFolder);
>+  var msgHdr = messenger.msgHdrFromURI(msgUri);
But then later you have
>+    var folderUri = messenger.getFolderUriAtNavigatePos(relPos);
>+    var msgUri = messenger.getMsgUriAtNavigatePos(relPos);
>+    dump("msg uri = " + msgUri.toString() + " folder Uri = "  + folderUri.toString() + "\n");
>+    // want to get rid of "-message:" and replace it with ":"
>+    var msgUriStr = new String("");
>+    msgUriStr = msgUri;
>+    msgUriStr.replace("-message:", ":");
>+    dump("msg uri = " + msgUriStr.toString +  "\n");
>+    var msgHdr = messenger.msgHdrFromURI(msgUriStr);
Which is right?
After this patch checked in, there are duplicate entities <!ENTITY forwardMsgCmd.label "Forward">. See 
http://lxr.mozilla.org/mozilla/source/mail/locales/en-US/chrome/messenger/messenger.dtd#282
and 
http://lxr.mozilla.org/mozilla/source/mail/locales/en-US/chrome/messenger/messenger.dtd#306

That looks wrong.
(Assignee)

Comment 19

11 years ago
Re Neil's question - I believe they both work, in which case the second one should be fixed. I'll see if that works.

Re the duplicate entity, I'll look into that as well.
(Assignee)

Comment 20

11 years ago
Created attachment 234697 [details] [diff] [review]
fix the dtd entities.

use consistent names for the go nav menu item entities.
Attachment #234697 - Flags: superreview?(sspitzer)
Comment on attachment 234697 [details] [diff] [review]
fix the dtd entities.

sr=sspitzer, acting sr for mailnews (while mscott is away)
Attachment #234697 - Flags: superreview?(sspitzer) → superreview+
(Assignee)

Comment 22

11 years ago
Created attachment 234707 [details] [diff] [review]
fix Neil's issue

remove unneeded code as pointed out by Neil.
Attachment #234707 - Flags: superreview?(sspitzer)
(Assignee)

Comment 23

11 years ago
fixed on trunk and branch (I've hijacked this bug from the suite - I don't know if the suite will want this or not...)
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Component: MailNews: Main Mail Window → Mail Window Front End
Keywords: fixed1.8.1
Product: Mozilla Application Suite → Thunderbird
Resolution: --- → FIXED
Target Milestone: Future → Thunderbird2.0
Comment on attachment 234707 [details] [diff] [review]
fix Neil's issue

sr=sspitzer, acting sr for mscott.
Attachment #234707 - Flags: superreview?(sspitzer) → superreview+

Comment 25

11 years ago
> I don't know if the suite will want this

Yes, we do. Luckily, attachment 233310 [details] [diff] [review] doesn't seem to break anything if this feature isn't used...

(Assignee)

Comment 26

11 years ago
no, it shouldn't break anything - unless you add the ui for the toolbar button or the menu item, none of the new code should be invoked.

Comment 27

11 years ago
Are these buttons supposed to work in the standalone message window?  They don't appear to be.
(Assignee)

Comment 28

11 years ago
I'll fix that - it's supposed to work...

Comment 29

11 years ago
One more thing: the "Go Back" button (and the corresponding menu item) isn't disabled, if there is no more item in history to go back.
(Assignee)

Comment 30

11 years ago
Created attachment 234936 [details] [diff] [review]
fix navigation in stand-alone msg window

this gets navigation working in a stand-alone msg window (for both normal folders and saved searches). I'll fix updating the enabling/disabling next.
Attachment #234936 - Flags: superreview?(mscott)
(Assignee)

Comment 31

11 years ago
Created attachment 234956 [details] [diff] [review]
fix enabling/disabling of forward/back buttons in stand-alone msg window
Attachment #234956 - Flags: superreview?(mscott)

Updated

11 years ago
Attachment #234936 - Flags: superreview?(mscott) → superreview+

Updated

11 years ago
Attachment #234956 - Flags: superreview?(mscott) → superreview+
(Assignee)

Comment 32

11 years ago
all patches so far landed on trunk and 2.0 branch

Comment 33

11 years ago
Hey David,

You'll want to wrap the key commands:

          <menuitem label="&goForwardCmd.label;" key="key_goForward" observes="cmd_goForward"/>
          <menuitem label="&goBackCmd.label;" key="key_goBack" observes="cmd_goBack"/>

in xp_macosx ifndefs otherwise you won't be able to type [ or ] in the quick search bar on the mac :)
(Assignee)

Comment 34

11 years ago
I tried quick search on [ and ] and it worked...ah, the mysteries of the mac. Have ou tried it?

Updated

11 years ago
Blocks: 360488
david: Using version 2 beta 1 (20070115, I don't see the functionality present in a standalone mail window (buttons are greyed out on toolbar and I can't navigate thru the Go menu). In the mail pane, it seems to work fine.
Sorry, forgot to mention I was running on Win XP, not the Mac.

(In reply to comment #35)
> david: Using version 2 beta 1 (20070115, I don't see the functionality present
> in a standalone mail window (buttons are greyed out on toolbar and I can't
> navigate thru the Go menu). In the mail pane, it seems to work fine.
> 

(Assignee)

Comment 37

11 years ago
Marcia, try this:

Select a folder with multiple unread messages.
double click on a message to open a stand-alone msg window
hit 'n' to go next unread.

Does the back button enable then? It does for me...

Updated

11 years ago
Depends on: 368177

Updated

11 years ago
Duplicate of this bug: 295552
verified fixed 1.8.1.3 using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.3) Gecko/20070326 Thunderbird/2.0.0.0 Mnenhy/0.7.5.0 ID:2007032620 (Thunderbird 2 RC1) and tested with david`s steps from comment #37
Keywords: verified1.8.1.3
Verified on 2.0.0.0 rc2 builds on Win XP and Mac OS 10.4.9
Status: RESOLVED → VERIFIED

Updated

11 years ago
Keywords: fixed1.8.1 → verified1.8.1
Depends on: 390921

Comment 41

10 years ago
Created attachment 282902 [details] [diff] [review]
SM port

Adaption for SM suiterunner
Attachment #282902 - Flags: superreview?(neil)
Attachment #282902 - Flags: review?(mnyromyr)

Comment 42

10 years ago
(In reply to comment #41)

> Created an attachment (id=282902) [details]

Application/octet-stream? Hm, don't know, why this happened.

Attachment #282902 - Attachment is patch: true
Attachment #282902 - Attachment mime type: application/octet-stream → text/plain

Updated

10 years ago
Blocks: 398081

Comment 43

10 years ago
Comment on attachment 282902 [details] [diff] [review]
SM port

I had the chance to prereview this before attaching, so just two remarks for others:
- we need new icons for goback/goforward, this patch uses the respective  browser button icons in Classic and those of reply/forward in Modern
- we still need fixes for bug 368178 and bug 398081.
Attachment #282902 - Flags: review?(mnyromyr) → review+

Comment 44

10 years ago
(In reply to comment #43)
> - we need new icons for goback/goforward, this patch uses the respective 
> browser button icons in Classic and those of reply/forward in Modern
Odd, why reply/forward for Modern?

Comment 45

10 years ago
Comment on attachment 282902 [details] [diff] [review]
SM port

>+      case "button_goForward":
>+      case "button_goBack":
>+      case "cmd_goForward":
>+      case "cmd_goBack":
>+        if (gDBView)
>+          enabled.value = gDBView.navigateStatus((command == "cmd_goBack" || command == "button_goBack") ? nsMsgNavigationType.back : nsMsgNavigationType.forward);
>+        return enabled.value;
Please split these up into Back/Forward.

>+       case "button_goForward":
>+       case "cmd_goForward":
>+        MsgGoForward();
>+        break;
>+      case "button_goBack":
>+      case "cmd_goBack":
>+        MsgGoBack();
>+        break;
Back before Forward again (and similarly throughout)

>+var gNavDebug = false;
>+function NavDebug(str)
>+{
>+  if (gNavDebug)
>+    dump(str);
>+}
Drop this.

>+  curPos.value = curPos.value * 2;
Please don't reuse this, instead have two separate variables.

>+  for (var i = curPos.value; isBackMenu ? (i >= 0) : (i < historyArray.length); i += (isBackMenu ? -2 : 2))
Even with the comment, this is too convoluted. Please put the back and forward cases in the _init functions (but you can leave the menuitem creation code below in a shared function).

>+    var menuText = "";    
>+    var msgHdr = messenger.msgHdrFromURI(historyArray[i]);
>+    if (!IsCurrentLoadedFolder(folder))
>+      menuText = folder.prettyName + " - ";
You don't use the msgHdr just yet, please declare it after this line.

>+    menuText += msgHdr.subject;
>+    menuText += ":";
>+    menuText += msgHdr.author;
These don't need to be separate concatenations. (Should we actually use the description attribute for the author?)

>+    relPos += isBackMenu ? -1 : 1;
Sigh, what a useless API we have to deal with :-(

>+    newMenuItem.setAttribute('oncommand', 'NavigateToUri(event.target); event.stopPropagation();');
Please define this on the menupopups in the XUL.

>+    if (! (relPos % 50))
50? That's rather a big menu... how about 15?

>+function forwardToolbarMenu_init(menuPopup)
>+{
>+  PopulateHistoryMenu(menuPopup, false);
>+}
This belongs directly after the back menu, not with the other stuff in between.

>+<!ENTITY % globalDTD SYSTEM "chrome://global/locale/global.dtd" >
>+%globalDTD;
Don't bother with this, we're nowhere near RTL-aware yet.

>       </menu>
>+          <menuitem label="&goForwardCmd.label;" key="key_goForward" observes="cmd_goForward"/>
>+          <menuitem label="&goBackCmd.label;" key="key_goBack" observes="cmd_goBack"/>
>       <menuseparator observes="mailHideMenus"/>
Indentation seems to be wrong here.

>+          <menuitem label="&goBackCmd.label;"
>+                    observes="cmd_goBack"/>
>+          <menuitem label="&goForwardCmd.label;"
>+                    observes="cmd_goForward"/>
These get deleted immediately. Don't bother with them.

>+#button-goforward {
Is there any chance you can use the browser buttons in the Modern theme?
Attachment #282902 - Flags: superreview?(neil) → superreview-

Comment 46

10 years ago
(In reply to comment #45)
> >+    if (! (relPos % 50))
> 50? That's rather a big menu... how about 15?

That is my fault/wish. The original TB patch has 20 items, which I feel are too *few*, so I recommended 50. 

> >+#button-goforward {
> Is there any chance you can use the browser buttons in the Modern theme?

Not really, they simple don't fit into the very special Modern mailnews theming, so I was fine with using the reply/forward icons here.

Comment 47

10 years ago
Created attachment 283929 [details] [diff] [review]
SM port, v2: addressed Neil's review comments

Hartmut did most of the changes Neil requested, but asked me to fill in the PopulateHistoryMenu stuff where he lacked intimate mailnews XUL/JS knowledge.

Hartmut, many thanks for putting together all these cumbersome details this patch requires!

Changes to hafi's patch:
- reordered any back/forward stuff, so that the back code is always before the forward one
- clean-up of the PopulateHistoryMenu method, so that I don't think that drawing the loop out into the Init functions is necessary anymore; also ported over a TB fix for MIME-encoded subjects and authors
Attachment #282902 - Attachment is obsolete: true
Attachment #283929 - Flags: superreview?(neil)

Comment 48

10 years ago
(In reply to comment #46)
>(In reply to comment #45)
>>>+    if (! (relPos % 50))
>>50? That's rather a big menu... how about 15?
>That is my fault/wish. The original TB patch has 20 items, which I feel are too
>*few*, so I recommended 50.
Well, I'm not convinced... I can't actually fit more than about 25 on my screen, and I shudder to think what BenoitRen will think ;-)

>>>+#button-goforward {
>>Is there any chance you can use the browser buttons in the Modern theme?
>Not really, they simple don't fit into the very special Modern mailnews
>theming, so I was fine with using the reply/forward icons here.
Well, I guess we could take the back/forward buttons, cut out the circle, and paste over the stop button (which we already have in both versions).

Comment 49

10 years ago
Comment on attachment 283929 [details] [diff] [review]
SM port, v2: addressed Neil's review comments

>Index: mailnews/base/resources/content/mail3PaneWindowCommands.js
>+      case "button_goBack":
>+      case "cmd_goBack":
>+        if (gDBView)
>+          enabled.value = gDBView.navigateStatus(nsMsgNavigationType.back);
>+        return enabled.value;
I prefer the versions of the code in messageWindow.js (back and forward).

>+       (i >= 0) && (i < maxPos) && (itemCount < 30);
Nit: the ()s aren't strictly necessary.

>+function NavigateToUri(target)
>+{
>+  var historyIndex = target.getAttribute('value');
>+  var folderUri = messenger.getFolderUriAtNavigatePos(historyIndex);
>+  var msgUri = messenger.getMsgUriAtNavigatePos(historyIndex);
>+  var folder = RDF.GetResource(folderUri).QueryInterface(Components.interfaces.nsIMsgFolder);
>+  var msgHdr = messenger.msgHdrFromURI(msgUri);
>+  messenger.navigatePos += Number(historyIndex);
>+  if (IsCurrentLoadedFolder(folder))
I had another look at the backend. Would this work?
newMenuItem.setAttribute('value', i);
...
messenger.navigatePos = target.getAttribute('value');
var msgUri = messenger.getMsgUriAtNavigatePos(0);
var msgHdr = messenger.msgHdrFromURI(msgUri);
if (IsCurrentLoadedFolder(msgHdr.folder))

The "Go Forward" button didn't have an icon at all in the Modern theme, did some of the style rules get misplaced somewhere? It gave me the chance to try using the navigation buttons, at least as placeholders; for forward I used:
list-style-image: url(chrome://navigator/skin/icons/btn1.gif);
-moz-image-region: rect(42px, 41px, 75px, 0px);
Attachment #283929 - Flags: superreview?(neil) → superreview+

Comment 50

10 years ago
(In reply to comment #49)
>var msgHdr = messenger.msgHdrFromURI(msgUri);
>if (IsCurrentLoadedFolder(msgHdr.folder))
No, beacuse this is a virtual folder, but it might be quicker to get the loaded folder's URI and do a string compare than get the resource of the URI.

Updated

10 years ago
Blocks: 399366

Comment 51

10 years ago
Created attachment 284384 [details] [diff] [review]
SM port, v3: checked-in version

(In reply to comment #48)
> >That is my fault/wish. The original TB patch has 20 items, which I feel are too
> >*few*, so I recommended 50.
> Well, I'm not convinced... I can't actually fit more than about 25 on my
> screen,

So 25 it is.
25 rows must be C= 64 style, right? ;-)

> Well, I guess we could take the back/forward buttons, cut out the circle, and
> paste over the stop button (which we already have in both versions).

Filed bug 399366 for the icons.

(In reply to comment #49)
> I prefer the versions of the code in messageWindow.js (back and forward).

Done.

> >+       (i >= 0) && (i < maxPos) && (itemCount < 30);
> Nit: the ()s aren't strictly necessary.

Yeah, but it's much more readable this way.

> The "Go Forward" button didn't have an icon at all in the Modern theme, did
> some of the style rules get misplaced somewhere?

Yes, sorry, left some stray list-style-image rule in the patch.

(In reply to comment #50)
> it might be quicker to get the loaded
> folder's URI and do a string compare than get the resource of the URI.

Done.


Taking over review flags.
Landed on trunk.
Attachment #283929 - Attachment is obsolete: true
Attachment #284384 - Flags: superreview+
Attachment #284384 - Flags: review+

Updated

10 years ago
No longer blocks: 360488
You need to log in before you can comment on or make changes to this bug.