Add Unsorted Bookmarks to Bookmarks menu and rename Personal Toolbar to Bookmarks Toolbar

RESOLVED FIXED in seamonkey2.9

Status

defect
--
minor
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)

Tracking

(Blocks 1 bug)

Trunk
seamonkey2.9
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Since the landing of Places-based bookmarks, Unsorted Bookmarks is available from the Bookmarks Manager and the Bookmarks sidebar. However it is missing from the Bookmarks menu. That's consistent with Firefox, but not consistent in itself. It should be added to the menu.
Depends on: SMPlacesBMarks
I'm not 100% sure how useful it is to have it around on either the bookmarks toolbar button or the bookmarks menu.

Marco, I probably copied the difference from the Firefox bookmarks toolbar button and menu, what specific reason is there for including it in one and not the other?
So, in bug 544817, the BMB_unsortedBookmarksFolderMenu was only ever added to the bookmarks button, but KaiRo must have copied the gnomestripe CSS file which referred to a bogus unsortedBookmarksFolderMenu.
(In reply to comment #1)
> Marco, I probably copied the difference from the Firefox bookmarks toolbar
> button and menu, what specific reason is there for including it in one and not
> the other?

That stuff is still being examinated to make it coherent based on scope, no obvious reasons.
I'll try to get this done, and rename remaining "Personal Toolbar" occurrences to "Bookmarks Toolbar" while I'm at it. Then, in yet another bug, I shall (try to) change the default File Bookmark(s) location to Unsorted Bookmarks. This madness needs to end.
Assignee: nobody → jh
Status: NEW → ASSIGNED
Posted patch patch (obsolete) — Splinter Review
The patch will also fix bug 588191.

Note: I intenionally left out Help since that's an extra-huge task, given that we have no documentation that takes Places-based bookmarks into account yet at all.
Attachment #587735 - Flags: review?(neil)
Comment on attachment 587735 [details] [diff] [review]
patch

This doesn't look as if it's the right way to set the label for the menu. Presumably we have to adapt BookmarksMenuButton from browser-places.js?
Attachment #587735 - Flags: review?(neil) → review-
Posted patch patch v2 (obsolete) — Splinter Review
(In reply to neil@parkwaycc.co.uk from comment #6)
> This doesn't look as if it's the right way to set the label for the menu.

I don't really see the point of lazily setting the label. Is it because the entry can be removed (didn't check whether you can)?

> Presumably we have to adapt BookmarksMenuButton from browser-places.js?

Done.
Attachment #587735 - Attachment is obsolete: true
Attachment #588175 - Flags: review?(neil)
(In reply to Jens Hatlak from comment #7)
> (In reply to comment #6)
> > This doesn't look as if it's the right way to set the label for the menu.
> I don't really see the point of lazily setting the label. Is it because the
> entry can be removed (didn't check whether you can)?
Probably it's to avoid duplicating the property entry (which is needed for the Bookmark Tree in the manager and sidebar).
Comment on attachment 588175 [details] [diff] [review]
patch v2

>+                 onpopupshowing="BookmarksMenu.onPopupShowing(event, '');
This gave me a shock until I realised that our existing references to BookmarksMenu are stale. Leaving them in will confuse people though ;-)
Attachment #588175 - Flags: review?(neil) → review+
Blocks: 588191
(In reply to neil@parkwaycc.co.uk from comment #9)
> >+                 onpopupshowing="BookmarksMenu.onPopupShowing(event, '');
> This gave me a shock until I realised that our existing references to
> BookmarksMenu are stale. Leaving them in will confuse people though ;-)

I'll address this and upload a new patch then.
Summary: Add Unsorted Bookmarks to Bookmarks menu → Add Unsorted Bookmarks to Bookmarks menu and rename Personal Toolbar to Bookmarks Toolbar
The removal of bookmarksPopupset, which was missed in bug 580660 (it even was in the context of its patch!), is the only change to v2.
Attachment #588175 - Attachment is obsolete: true
Attachment #588545 - Flags: review?(neil)
Attachment #588545 - Flags: review?(neil) → review+
Comment on attachment 588545 [details] [diff] [review]
patch v2a [Checkin: Comment 12]

http://hg.mozilla.org/comm-central/rev/d29573dcbadb
Attachment #588545 - Attachment description: patch v2a → patch v2a [Checkin: Comment 12]
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.9
You need to log in before you can comment on or make changes to this bug.