Closed Bug 41345 Opened 26 years ago Closed 26 years ago

[FEATURE] There is no checkmark for the current page in the 'Go' menu

Categories

(Core :: DOM: Navigation, defect, P1)

Product:
Core
Component:
DOM: Navigation
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: cmaximus, Assigned: bugzilla)

References

Details

(Whiteboard: [nsbeta2-])

***Overview Description: In the session history list under the 'Go' menu, there is no checkmark next to the current page to help indicate where you currently are within the list. This is visually disorienting and makes the feature more difficult to use. ***Steps to Reproduce: 1) Surf a few pages. Select 'Go' ***Actual Results: A list of recently visited sites (SH) is displayed, but it's difficult to tell where within hte lsit oyu currently are. No checkmark. ***Expected Results: Please put a check next to the current page. ***Build Date & Platform Bug Found: All platforms with the 2000060208 builds
This is a small feature work.
Status: NEW → ASSIGNED
Summary: There is no checkmark for the current page in the 'Go' menu → [feature]There is no checkmark for the current page in the 'Go' menu
Target Milestone: --- → M17
Yep, we definitely forgot to implement this. Nice catch, Claudius. Nominating for beta 2 since it IS a feature. My recommendation however is that PDT should mark this nsbeta- since it's such a minor thing.
Keywords: nsbeta2
Putting on [nsbeta2-] radar.
Keywords: nsbeta3
Whiteboard: [nsbeta2-]
Could we get PDT love if we have a fix in hand? I think I know someone who might. cc'ing blaker1234@aol.com who just submitted a patch for a similar bug, pardons for forcefully volunteering blake by the way :-)
Radha, do we have any kind of backend implemented for this yet? has any work been done towards implementing this feature?
I haven't done anything on this one. I think the job involves changing the xul/js to use the radiobox menuitem and enable it at the right index when th go menu is clicked on.
radha, I'm going to work on this...would you mind kicking me in the direction of where the menuitems are created and put onto the Go menu? I see similar functions in sessionhistoryUI.js, but that looks like it's for the URL bar dropdown. let me know, thanks!
I notice nsBrowserInstance::UpdateGoMenu -- should I be lookin' for this in the C++ or JS?
Thanks for looking in to this. maybe I can hand off little more cleanup task related to this, to you. Currently updategomenu calls in to appCore.updateGoMenu which basically calls nsBrowserAppCore::UpdateGoMenu. I want to obsolete this and all other history menu creation/deletion code in nsBrowserAppCore and move them to JS. The good news is, the JS Code is also in there in sessionHistoryUI.JS. Look in to FillHistoryMenu() in sessionHistoryUI.js. FillHistoryMenu() caters to menu creation in Back/forward and Go menu. The task involves 1) Modifying the menu creation code for 'Go' menu to a radio box menuitem. Maybe a parameter to CreateMenuItem() 2) Change updategomenu(), BrowserBackMenu() and BrowserForwardMenu() in navigator.js to call FillHistoryMenu() 3) Make sure everything works right with the new code. There is a small bug in the menu creation/deletion code. If you notice, Back/forward and Go menus don't have a onDestroy handler that will destroy the menu items when it goes away. Instead old menu items are deleted just before creating new ones in FillHistoryMenu(). This is because there is a bug with menu destruction code and the browser crashes if I put a onDestroy handlers for these menus. saari has a bug on this. http://bugzilla.mozilla.org/show_bug.cgi?id=37301. Let me know if you need more info. Thanks,
radha - thanks for the info, very useful. I'll start playing around with it and contact you if I have any questions. Thanks!
reassigning to me, I'm inches from this.
Assignee: radha → BlakeR1234
Status: ASSIGNED → NEW
As long as I'm down in history stuff, it wouldn't be too hard to add the numeric access keys that nav4 uses. For example: _0: Mozilla.org _1: anyplace.com and so forth. Don, do we plan on doing that (i.e. should I)?
This is working now with the exception of an occasional crash. Once I find out and fix where the crash is occurring, hopefully we can get a little nsbeta2+ exception feature fun here...
Status: NEW → ASSIGNED
Let me know when it's ready, tested, and safe, and then I'll ask the PDT to consider it.
awaiting response back from radha (np -- I know she's got lots more important things to work on -- just updating this bug)
Summary: [feature]There is no checkmark for the current page in the 'Go' menu → [FEATURE] There is no checkmark for the current page in the 'Go' menu
*** Bug 44715 has been marked as a duplicate of this bug. ***
Priority: P3 → P1
Target Milestone: M17 → M18
Fix checked in. Thanks for the guidance, radha.
Status: ASSIGNED → RESOLVED
Closed: 26 years ago
Keywords: 4xp
Resolution: --- → FIXED
This is fast becoming a wontfix because of all the problems it's causing this late in the game...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
fixed..
Status: REOPENED → RESOLVED
Closed: 26 years ago → 26 years ago
Resolution: --- → FIXED
*** Bug 49676 has been marked as a duplicate of this bug. ***
*** Bug 49704 has been marked as a duplicate of this bug. ***
VERIFIED Fixed 2000083008 builds
Status: RESOLVED → VERIFIED
Component: History: Session → Document Navigation
QA Contact: claudius → docshell
You need to log in before you can comment on or make changes to this bug.