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)
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)
(deleted),
patch
|
mconnor
:
review+
mconnor
:
approval-branch-1.8.1+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mconnor
:
review+
mconnor
:
approval-branch-1.8.1+
|
Details | Diff | Splinter Review |
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.
Comment 1•19 years ago
|
||
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
Comment 2•19 years ago
|
||
See Bug 260268.
Comment 3•19 years ago
|
||
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.
Comment 4•19 years ago
|
||
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.
Comment 5•19 years ago
|
||
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.
Comment 6•19 years ago
|
||
(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.
Comment 7•19 years ago
|
||
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...
Comment 8•19 years ago
|
||
(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.
Comment 9•19 years ago
|
||
*** Bug 337032 has been marked as a duplicate of this bug. ***
Assignee | ||
Updated•18 years ago
|
Assignee: nobody → looney.liz
Assignee | ||
Comment 10•18 years ago
|
||
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?
Assignee | ||
Updated•18 years ago
|
Whiteboard: SWAG: 1
Comment 11•18 years ago
|
||
If all that is changing is the name, then I think that the ID should not change.
Assignee | ||
Comment 12•18 years ago
|
||
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?
Assignee | ||
Updated•18 years ago
|
Attachment #223971 -
Flags: review? → review?(mconnor)
Assignee | ||
Comment 13•18 years ago
|
||
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)
Updated•18 years ago
|
Attachment #223986 -
Flags: review?(mconnor)
Attachment #223986 -
Flags: review+
Attachment #223986 -
Flags: approval-branch-1.8.1+
Comment 14•18 years ago
|
||
(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".
Comment 15•18 years ago
|
||
(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.
Comment 16•18 years ago
|
||
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.
Comment 17•18 years ago
|
||
Attachment 223986 [details] [diff] checked in on the branch and trunk.
Comment 18•18 years ago
|
||
Please consider renaming "History" to "View History" in the History menu. It's confusing now.
Comment 19•18 years ago
|
||
*** Bug 331829 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 20•18 years ago
|
||
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)
Updated•18 years ago
|
Attachment #224254 -
Flags: review?(mconnor)
Attachment #224254 -
Flags: review+
Attachment #224254 -
Flags: approval-branch-1.8.1+
Assignee | ||
Comment 21•18 years ago
|
||
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
Comment 22•18 years ago
|
||
I've landed patch 3 on the branch and trunk.
Status: REOPENED → RESOLVED
Closed: 18 years ago → 18 years ago
Resolution: --- → FIXED
Updated•18 years ago
|
Whiteboard: SWAG: 1, checkin needed → SWAG: 1
You need to log in
before you can comment on or make changes to this bug.
Description
•