Closed Bug 336058 Opened 19 years ago Closed 18 years ago

Use History as a top level menu item instead of Go

Categories

(Firefox :: Menus, defect)

2.0 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 2 beta1

People

(Reporter: kbrosnan, Assigned: looney.liz)

References

Details

(Keywords: fixed1.8.1, Whiteboard: SWAG: 1)

Attachments

(2 files, 1 obsolete file)

Places used History as a top level menu item. The recent reverting to the old style of bookmarks in the 1.8 branch changed the menu item back to Go. I think it would be good to change it back to History.
Yeah, that's part of the chrome polishing stuff, ccing beltzner, slotting into beta1 since its not critical to getting an alpha done.
Flags: blocking-firefox2+
Target Milestone: --- → Firefox 2 beta1
I wonder if people who have their bookmark toolbar items on the menubar wouldn't have problems with this change to a longer name. Especially in the Dutch localization (change from "Ga" to "Geschiedenis") it would cost you space for two bookmarks.
I'm not especially worried about that case, its edge-to-cornerish. Moving the bookmarks toolbar works, but we're not going to compromise on primary UI to support better optimization for space.
Go is a simple tradition going back prior to Netscape 3. I like it. OTOH, seeing a menuitem named history at the bottom of an open menu item named history would look really dumb.
(In reply to comment #4) > One should really keep a poll for this kind of changes. Because things that you lose have much more impact than things that you gain.
Polls (and any other form of democratic voting) are a terrible way to make design decisions. That isn't how we do things, or have ever done them. If there's a real reason, aside from vague concerns about toolbar space, please do voice that reason. I certainly didn't see any negative feedback about the change in a1...
(In reply to comment #7) > Polls (and any other form of democratic voting) are a terrible way to make > design decisions. > No, I don't think that you can make very wrong decisions based of right questions and right conclusions.
*** Bug 337032 has been marked as a duplicate of this bug. ***
Assignee: nobody → looney.liz
In browser/base/content/browser-menubar.inc, the Go menu is defined. The element id is "go-menu". Do you think the element id should also be changed to history-menu? I can see pros: New extension developers who want to add menuitems to the "History" menu might be confused if the "History" menu has the id "go-menu". and cons: Experienced extension developers who want to add menu items might be annoyed that the id is no longer "go-menu". My thought on this is to leave the element id as "go-menu". Any arguments against that?
Whiteboard: SWAG: 1
If all that is changing is the name, then I think that the ID should not change.
Attached patch Patch 1 (obsolete) (deleted) — Splinter Review
Here's a patch that changes browser-menu.inc to use entities &historyMenu.label; and &historyMenu.accesskey; instead of &goMenu.label; and &goMenu.accesskey;. It also changes browser.dtd to move the historyMenu.label and historyMenu.accesskey entities out of the "places only" section and adds a comment saying that they are used with or without places.
Attachment #223971 - Flags: review?
Attachment #223971 - Flags: review? → review?(mconnor)
This patch has the same changes to browser-menu.inc and browser.dtd as patch #1. This patch also has changes to 3 help files that discuss the Go (now History) menu: firebird-toc.rdf, menu_reference.xhtml, and using_firebird.xhtml.
Attachment #223971 - Attachment is obsolete: true
Attachment #223986 - Flags: review?(mconnor)
Attachment #223971 - Flags: review?(mconnor)
Attachment #223986 - Flags: review?(mconnor)
Attachment #223986 - Flags: review+
Attachment #223986 - Flags: approval-branch-1.8.1+
(In reply to comment #11) > If all that is changing is the name, then I think that the ID should not > change. > If this changes the name to match the trunk, then I think the ID should also match whatever is on trunk, so that extension developers writing something for Fx2 will have it work rather than break strangely in Fx3. On trunk, this menu ID is "history-menu".
(In reply to comment #14) > If this changes the name to match the trunk, then I think the ID should also > match whatever is on trunk, so that extension developers writing something for > Fx2 will have it work rather than break strangely in Fx3. I'd rather keep it so that extensions from Firefox 1.5 don't needlessly break - especially considering that between 1.5 and 2.0 we probably have just a label change, whereas between 2.0 and 3.0 there are code changes which will much rather break an extension.
OK, I'll concede that. Still seems like the anchors in the help file should be #history instead of #go but that's really just nitpicky :) I do agree with comment #5 that "History" is a poor choice for a menu item in "History". (History->History?) Perhaps "View History" or "View all History" (closer to trunk) would be better.
Attachment 223986 [details] [diff] checked in on the branch and trunk.
Status: NEW → RESOLVED
Closed: 18 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Please consider renaming "History" to "View History" in the History menu. It's confusing now.
*** Bug 331829 has been marked as a duplicate of this bug. ***
On #developers, beltzner suggested "Show in Sidebar" instead of "View History" and others agreed. Patch #3 changes History->History to History->Show in Sidebar. It also updates the related help files. This was a little tricky because "History" was the label of a broadcaster that is observed from several other elements, such as the View->Sidebar->History menuitem and the History toolbarbutton. So, I removed the label attribute from the broadcaster and put it on each of the observers, using a different label for the menuitem on the History menu. Then, I added a sidebartitle attribute to the broadcaster, because that attribute is used in the toggleSidebar function (in browser.js) to set the title displayed on the sidebar. (If the sidebartitle attribute is not present, it will use the label, but now that's not present.)
Attachment #224254 - Flags: review?(mconnor)
Attachment #224254 - Flags: review?(mconnor)
Attachment #224254 - Flags: review+
Attachment #224254 - Flags: approval-branch-1.8.1+
We just need patch 3 landed and then this bug will be fixed.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: SWAG: 1 → SWAG: 1, checkin needed
I've landed patch 3 on the branch and trunk.
Status: REOPENED → RESOLVED
Closed: 18 years ago18 years ago
Resolution: --- → FIXED
Whiteboard: SWAG: 1, checkin needed → SWAG: 1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: