599731 - Move "Recently Closed" menus to Go menu

Status: RESOLVED FIXED

(SeaMonkey :: Bookmarks & History, enhancement)

Description

[Aki Laaksovirta]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US) AppleWebKit/534.3 (KHTML, like Gecko) Chrome/6.0.472.63 Safari/534.3
Build Identifier: 

I think that the "Recently Closed Tabs" and the "Recently Closed Windows" menus should be moved from the File menu to the Go menu.The reasons:

* They are not related to the current page, but they are rather a part of the "browsing history" in the broader sense.

*The File menu typically contains items that are related to opening or saving files.

Reproducible: Always

Comment 1

[Misak Khachatryan]

I'm working on porting on demand restore session, deciding where to place it's menu entry. My first though is File menu, bug i decided to place it to Go menu. Also i propose rename Go menu to History, which will better reflect it's entries.

Comment 2

[Aki Laaksovirta]

> Also i propose rename Go menu to History, which will better reflect it's
> entries.

I agree, but you should probably file a separate bug about that.

Comment 3

[Jens Hatlak (:InvisibleSmiley)]

Valid request, so confirming. Doesn't need to be decided now, though.

Comment 4

[Edmund Wong (:ewong)]

Attachment: Moved "Recently Closed" menus to Go Menu.

Comment 5

[Ian Neal]

Comment on attachment 544465 [details] [diff] [review]
Moved "Recently Closed" menus to Go Menu.

>+++ b/suite/browser/navigatorOverlay.xul

>+        <menu label="&recentTabs.label;"
>+              accesskey="&recentTabs.accesskey;"
>+              id="menu_recentTabs">
Nit: I'd prefer the id to be first.
>+          <menupopup id="menu_recentTabsPopup"
>+                     onpopupshowing="updateRecentTabs(this);"
>+                     oncommand="gBrowser.undoCloseTab(event.target.value);"/>
>+        </menu>
>+        <menu label="&recentWindows.label;"
>+              accesskey="&recentWindows.accesskey;"
>+              id="menu_recentWindows">
Nit: id first please

>+++ b/suite/locales/en-US/chrome/browser/navigator.dtd
>@@ -14,22 +14,16 @@
>+<!ENTITY recentTabs.label "Recently Closed Tabs">
>+<!ENTITY recentTabs.accesskey "R">
You will have to change this accesskey, perhaps "C", as "Restore Previous Session" already uses "R".

>+<!ENTITY recentTabs.commandkey "t">
>+<!ENTITY recentWindows.label "Recently Closed Windows">
>+<!ENTITY recentWindows.accesskey "y">
>+<!ENTITY recentWindows.commandkey "y">
> <!ENTITY historyRestoreLastSession.label "Restore Previous Session">
> <!ENTITY historyRestoreLastSession.accesskey "R">

Does help need changing to reflect this new menu structure?

Comment 7

[Edmund Wong (:ewong)]

(In reply to comment #5)
> Does help need changing to reflect this new menu structure?

Filed bug #670513.

Comment 8

[Edmund Wong (:ewong)]

Attachment: Moved "Recently Closed" menus to Go Menu.(v2) [Checked in: Comment 9]

Comment 9

[Ian Neal]

Comment on attachment 545069 [details] [diff] [review]
Moved "Recently Closed" menus to Go Menu.(v2) [Checked in: Comment 9]

http://hg.mozilla.org/comm-central/rev/b1075fd310df

Comment 10

[[e-mail]]

(In reply to comment #5)
>(From update of attachment 544465 [details] [diff] [review])
>>+<!ENTITY recentTabs.label "Recently Closed Tabs">
>>+<!ENTITY recentTabs.accesskey "R">
>You will have to change this accesskey, perhaps "C", as "Restore Previous
>Session" already uses "R".
Why not "T"?

Would it make sense to group all the session restore items together?

Comment 11

[Ian Neal]

(In reply to comment #10)
> (In reply to comment #5)
> >(From update of attachment 544465 [details] [diff] [review] [diff] [details] [review])
> >>+<!ENTITY recentTabs.label "Recently Closed Tabs">
> >>+<!ENTITY recentTabs.accesskey "R">
> >You will have to change this accesskey, perhaps "C", as "Restore Previous
> >Session" already uses "R".
> Why not "T"?
I do keep forgetting that "T" has a fairly wide underscore when an accesskey.
> 
> Would it make sense to group all the session restore items together?
Indeed that is an option, could both be done in a spin off bug...

Comment 12

[[e-mail]]

(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #5)
> > >(From update of attachment 544465 [details] [diff] [review])
> > >>+<!ENTITY recentTabs.label "Recently Closed Tabs">
> > >>+<!ENTITY recentTabs.accesskey "R">
> > >You will have to change this accesskey, perhaps "C", as "Restore Previous
> > >Session" already uses "R".
> > Why not "T"?
> I do keep forgetting that "T" has a fairly wide underscore when an accesskey.
> > Would it make sense to group all the session restore items together?
> Indeed that is an option, could both be done in a spin off bug...
Of course. I only mention it because I had to build Firefox to debug bug 671466 and I liked the order that they've put their menuitems in.

Comment 13

[Edmund Wong (:ewong)]

(In reply to comment #11)

> > Would it make sense to group all the session restore items together?
> Indeed that is an option, could both be done in a spin off bug...

Spun off bug #675589.

Comment 14

[[e-mail]]

The code to enable and disable the menus was unfortunately hidden away at the end of updateCloseItems. This needs to be moved into a new function so that it can be called from the go popup.

Comment 15

[Ian Neal]

The relevant code to be moved starts at http://mxr.mozilla.org/comm-central/source/suite/browser/navigator.js#1500
You will need to add a onpopupshowing to the "history-menu" http://mxr.mozilla.org/comm-central/source/suite/browser/navigatorOverlay.xul#380
This could probably be done in bug 675672 as part of that work.

Comment 16

[Jens Hatlak (:InvisibleSmiley)]

added bug 599731 / bug 702297 to Known Issues

Checking in seamonkey2.5/index.en.html;
/www/seamonkeyproject-org/src/releases/seamonkey2.5/index.en.html,v  <--  index.en.html
new revision: 1.10; previous revision: 1.9
done