Closed Bug 17333 Opened 25 years ago Closed 25 years ago

Manage Bookmarks context menus should include properties

Categories

(SeaMonkey :: Bookmarks & History, defect, P1)

defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: fornixon, Assigned: mozilla)

References

Details

(Whiteboard: [PDT-])

Overview Description: The context menus for bookmarks in the Manage Bookmarks window do not contain a properties item Steps to Reproduce: 1) From browser, click Bookmarks|Manage Bookmarks 2) Expand a folder to expose some individual bookmark items 3) right-click a bookmark Actual Results: no properties item appears in the menu, user is forced to use main edit>properties item Expected Results: expect properties to appear in context menu Build Date & Platform Bug Found: Build ID 1999102608, Win98 Additional Builds and Platforms Tested On: none Additional Information: none
Assignee: don → german
Component: Browser-General → XPApps
QA Contact: leger → claudius
Reproduced using 1999102810 Mac OS build; guessing appropriate component & engineering/QA contacts. fornixon@globalserve.net, thanks for using the bug template!
Assignee: german → don
Yep there will definitely be a properties dialog esp. for each bookmark accessible through the context menu. Is this considered dogfood? Forwarding to Don, so he can assign to the proper engineer. Don if you need help with the props dialog feel free to re-assign to me. It should fairly similar to 4.x.
Assignee: don → slamm
Target Milestone: M14
Steve, is this fixable for beta 1?
Component: XPApps → Bookmarks
fornixon@globalserve.net is now the-enigman@home.com. CC'ing the new address to ensure I am notified of all changes to this bug.
We really need this fixed for beta 1 or the bookmarks are just not manageable.
Keywords: beta1
Priority: P3 → P1
marking PDT+ but would like product marketing to re-review.
Whiteboard: [PDT+]
clearing PDT+ as this is not a stop beta in Product Mkt's view. Back to PDT for consideration.
Whiteboard: [PDT+]
Whiteboard: [PDT-]
*** Bug 28238 has been marked as a duplicate of this bug. ***
*** Bug 28425 has been marked as a duplicate of this bug. ***
Summary: Manage Bookmarks context menus → Manage Bookmarks context menus should include properties
OK, I'm dumb. I've been looking at this and would fix it, but I can't find where fillContextMenu() in bookmarks.js gets its data from. A little help?
The menuitems come from the call to GetAllCmds. This resolves down to nsBookmarksService::GetAllCmds inside \xpfe\componenets\bookmarks\src\nsBookmarksService.cpp. I tried to fix this for you, but couldn't. In 4.x, looks like they put in a menuseparator and the Bookmark Properties menuitem regardless of the type of item selected. If it is a separator, then it disables the Bookmarks Properties menu item. I got all of this to work in bookmarks.js, but I couldn't figure out how to set the value of the bookmark Properties menu item to be localizable (i.e. from a DTD instead of a hardcoded string). Maybe someone else can figure this out. Here is what I had: /* put in a seperator between the other menuitems and the Bookmark * Properties menu item */ var menuItemSep = document.createElement("menuseparator"); popupNode.appendChild(menuItemSep); /* put in Bookmark Properties menu item */ var menuItem = document.createElement("menuitem"); menuItem.setAttribute("value", "crap"); menuItem.setAttribute("oncommand", "return BookmarkProperties();"); var type = select_list[0].getAttribute('type'); if (type == "http://home.netscape.com/NC-rdf#BookmarkSeparator") { /* disable the Bookmark Properties menuitem if the currently selected * item is a separator */ menuItem.setAttribute("disabled", "true" ); } popupNode.appendChild(menuItem); Which I put after the other menu items were put in the menu. Of course, my string of "crap" needs to be replaced by the localizable string. Also, might need to handle the case if a break was hit in the for loop that inserts items into the menu. I don't know if we want to put in this menuitem if there is nothing else in the menu.
aaronr, I've checked in your JavaScript (with slight changes): instead of disabling, only add the "Properties..." context menu is a single node is selected, and only if it is specifically a bookmark or a bookmark folder (check for these specifically, instead of adding it if it isn't a bookmark separator, as there can be other types). All that remains is getting the localized string from the bookmark's string bundle.
I put this out on the xpfe newsgroup and the latest suggestion is to try to get the string from the pulldown menuitem via DOM. I'll try that out and see how it works. I thought about applying it only if a single item was selected, like you said, but 4.x didn't work that way so I didn't make it that way either. I like your idea better. No sense only putting up one properties dialog if multiple items are selected.
For localization, we just need to create a string bundle via JavaScript which references the correct string file. (This is done in other areas of Mozilla, including the search sidebar panel I believe.)
I didn't see the changes you checked in. Did you check them into mozilla or just your netscape branch? In case you haven't done the string bundle stuff, yet, this worked for me (using the code I suggested earlier): In bookmarks.xul, add: <html:script src="chrome://global/content/strres.js"/> In bookmark.properties, add: bookmarkProperties = Bookmark Properties... In bookmarks.js, add: // initialise the string bundle. var bundle = srGetStrBundle("chrome://bookmarks/locale/bookmark.properties"); menuItem.setAttribute("value", bundle.GetStringFromName("bookmarkProperties")); I'd send the diffs, but I don't have your changes. --Aaron
The changes went into the tip (not the beta1 branch). I'll look at adding in your string bundle code later today or tomorrow. :^)
Robert, you seem to be doing all the work on this one ... :-)
Assignee: slamm → rjc
Target Milestone: M14 → ---
Fixed (for both the manage bookmarks window as well as the bookmark sidebar panel.) Thanks, Aaron.
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
VERIFIED Fixed
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.