Closed
Bug 17333
Opened 25 years ago
Closed 25 years ago
Manage Bookmarks context menus should include properties
Categories
(SeaMonkey :: Bookmarks & History, defect, P1)
SeaMonkey
Bookmarks & History
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
Updated•25 years ago
|
Assignee: don → german
Component: Browser-General → XPApps
QA Contact: leger → claudius
Comment 1•25 years ago
|
||
Reproduced using 1999102810 Mac OS build; guessing appropriate component &
engineering/QA contacts.
fornixon@globalserve.net, thanks for using the bug template!
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.
Updated•25 years ago
|
Component: XPApps → Bookmarks
Comment 4•25 years ago
|
||
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+]
Updated•25 years ago
|
Summary: Manage Bookmarks context menus → Manage Bookmarks context menus should include properties
Comment 10•25 years ago
|
||
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?
Comment 11•25 years ago
|
||
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.
Assignee | ||
Comment 12•25 years ago
|
||
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.
Comment 13•25 years ago
|
||
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.
Assignee | ||
Comment 14•25 years ago
|
||
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.)
Comment 15•25 years ago
|
||
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
Assignee | ||
Comment 16•25 years ago
|
||
The changes went into the tip (not the beta1 branch).
I'll look at adding in your string bundle code later today or tomorrow. :^)
Comment 17•25 years ago
|
||
Robert, you seem to be doing all the work on this one ... :-)
Assignee: slamm → rjc
Target Milestone: M14 → ---
Assignee | ||
Comment 18•25 years ago
|
||
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
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•