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

RESOLVED FIXED in seamonkey2.9

Status

SeaMonkey
Bookmarks & History
--
minor
RESOLVED FIXED
7 years ago
6 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)

(Assignee)

Description

7 years ago
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.
(Assignee)

Updated

7 years ago
Depends on: 498596

Comment 1

7 years ago
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?

Comment 2

7 years ago
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.
(Assignee)

Comment 4

6 years ago
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
(Assignee)

Comment 5

6 years ago
Created attachment 587735 [details] [diff] [review]
patch

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 6

6 years ago
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-
(Assignee)

Comment 7

6 years ago
Created attachment 588175 [details] [diff] [review]
patch v2

(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)

Comment 8

6 years ago
(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 9

6 years ago
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+
(Assignee)

Updated

6 years ago
Blocks: 588191
(Assignee)

Comment 10

6 years ago
(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
(Assignee)

Comment 11

6 years ago
Created attachment 588545 [details] [diff] [review]
patch v2a [Checkin: Comment 12]

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)

Updated

6 years ago
Attachment #588545 - Flags: review?(neil) → review+
(Assignee)

Comment 12

6 years ago
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]
(Assignee)

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.9
You need to log in before you can comment on or make changes to this bug.