215235 - Show bookmarks manager in tabs

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

[Jasper]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.5a) Gecko/20030603 Camino/0.7+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.5a) Gecko/20030603 Camino/0.7+

I'd like to request the option for a user to define weather he/she want's the
bookmarks manager to open in a new tab or in a new window.

This would offer great benefits over the cover up in-window method used now.
Allowing the user to open the bookmarks manager in a new window or tab would
bring back some of the drag and drop ability that was removed when discontuing
the sidebar.

Reproducible: Always

Steps to Reproduce:



Expected Results:  
Preferences
- the user is able to define weather they want the bookmarks manager to open in
a window or in a new tab.

Window
- When clicking the open bookmarks manager button in the toolbar a new window
open containing the bookmarks manager (the window does not have a toolbar,
bookmarks toolbar or status bar). When clicking the toolbar button again the
window is closed.
- When the bookmarks manager window is in the background clicking the bookmarks
toolbar button again closes the window.
- When closing the bookmarks  window (using the close widget in the left window
corner) is the same action as clicking the bookmarks toolbar button again.

Tab
- When clicking the open bookmarks manager button in the toolbar a new tab opens
containing the bookmarks manager (offering the user the capability to drag and
drop bookmarks to and from other tabs). When clicking the toolbar button again
the tab is closed.
- When the bookmarks manager tab is in the background clicking the toolbar
button again closes the tab.
- Closing the bookmarks tab (using the close tab tolbar button) is the same
action as clicking the bookmarks toolbar button again.

Comment 1

[Mike Pinkerton (not reading bugmail)]

things to think about

Comment 2

[Janus]

I personally prefer tab.

i would even like if there is an option to force open in a tab when the link is
specified as open in new window.

Comment 3

[Jasper]

Janus, please look at Bug 185280 for the force in tab request.

Comment 4

[Mike Pinkerton (not reading bugmail)]

not happening for 0.8 i'm afraid.

Comment 5

[Jasper]

Since so many people still want the sidebar back (in slimmed down basic editing
version) we could even add an additional preference where users could say they
want the bookmark manager to open in the sidebar!

The sidebar would be a list view with only bookmarks names, and has basic
editing capabilities such as rename/drag and drop/remove and such without buttons.

Just an idea.

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Found this bug and I want to express my support for either (or both) of these
options; the current "Bookmarks Manager" UI is terribly confusing.  

It "wipes out" all of one's current tabs (or the active page) and one has no
idea how to get back to them (i.e., no obvious visual clue; I eventually
discovered "Back" and "cmd-B" work, but it's not obvious, and counter-intuitive
for anyone coming from the older "bookmarks-manager-as-separate-window model".
Safari is as bad but at least has that permanent widget that toggles and
provides visual clues.)  

Either opening a new tab or new window for the Manager would really help clear
this up, since either would bring a "close" widget :-)

Comment 7

[Bruce Davidson]

Although this isn't a democracy I'm going to put a vote in for going down the
window route. This has several advantages over being in a tab:

i. We can use an appopriate toolbar for managing bookmarks etc, with proper
icons rather than the very small monochrome icons we use in the current bookmark
manager. We don't need to display the irrelevant browsing toolbar (what does
"reload" do when you're browsing bookmarks?). (If the user can hide the
"playlist" bit of the window, it works at drawer-like sizes quite happily.)

ii. If we remember the size and shape of our bookmark window separately from
browsing windows users who want it and have the screen estate can size and
position it like the old drawer, without causing the next browser they open to
be unusably narrow.

iii. Its instinctive that you can only have one instance of a bookmarks window
(in the same way you only have one instance of the downloads window), saving a
large potential set of confusion and bugs when users have the bookmarks window
open in several windows.

The largest drawback I can see is that its not clear what window/tab we open a
bookmark in when you double click it in the bookmark window, the "current"
tab/window may not be instinctively clear to the user, and always opening a new
tab/window is a bit wasteful.

Comment 8

[Jasper]

I think that we should stick doing it simple way. I think the bookmark manager
placement should be just as if it was a website. Clicking the icon would opene
it in the current window or tab and would allow back and forward just like
people are used to. I see no need to teach people a whole new way of doing this.

I think we should avoid alwaysopening a new window as I think we should always
push single window browsing where possible. After that we coudl think about
allowing user to open it in either a new tab or window if wished.

Comment 9

[Mike Pinkerton (not reading bugmail)]

if dave is going to look at doing this, we should do it through the url loading
mechanism, so we have a url like bookmarks: or about:bookmarks. that way, it can
be included in tabsets and made your home page if you desire.

Comment 10

[louis bennett]

If we're going for any sort of consistency across browsers, OmniWeb uses "bookmarks:/".

Comment 11

[Simon Fraser [no longer active]]

In Safari, the bookmarks view is also included in session history, which means
you see it when going Back or Forward. This also argues for using a special kind
of URL.

However, I think this is rather subtle, and not what most users expect.
Bookmarks are MINE on MY machine, not web pages out there. They shouldn't be
intertwingled with web sites.

Comment 12

[Jasper]

I agree with simon that the bm mamanger shouldn't interfere with history. But I
really do think that the bahaviour of the window should be that of a webpage. 

Comment 13

[Simon Fraser [no longer active]]

Taking this for bookmarks in tabs.

Comment 14

[Simon Fraser [no longer active]]

Attachment: Proof of concept patch.

First-cut patch. It's amazing how easy this was:
1. Create "about" handler module for "about:bookmarks" (otherwise
about:bookmarks doesn't do anything)
2. Allow clients to regiser custom views for particular URLs on BrowserWrapper,
which swaps in said views when those urls load.
3. There is no step 3.

Comment 15

[Simon Fraser [no longer active]]

Attachment: Fairly complete patch

This patch is almost fully-functional. Some minor issues remain:
* What to show in the url bar when bookmarks or history are showing
* Should there be a way to toggle bookmarks, or do we just treat them like a
page (which the current code does)

Comment 16

[Jasper]

(In reply to comment #15)
> This patch is almost fully-functional. Some minor issues remain:
> * What to show in the url bar when bookmarks or history are showing

Safari has a "nice" approach where it says in disabled text "Go to this address"
in the url bar. I find it a bit strange but looks nice. I don't think we should
do that.
Omniweb just displays "bookmarks:/" in the url bar. Clean and simple.

I guess just showing "about:bookmarks" with perhaps a small bookmarks book icon
as site icon would be the best an simplest. Note we also would want the page and
tab title to be "Bookmarks".

Note that Omniweb handles the bm manager as a page enabling usres to drag and
drop the url from the url bar in their bookmark bar so they can easily acces it
from there. I like this approach very much as it's very natural, people already
know how things would work if we did this why would we want them to learn a
whole new thing?

> * Should there be a way to toggle bookmarks, or do we just treat them like a
> page (which the current code does)

I think we should keep the bookmark toggle by using the key command or the
toolbar icon. Which basically means that you would go back and forward depending
on the bm manager being open or not. 

Comment 17

[Simon Fraser [no longer active]]

> I think we should keep the bookmark toggle by using the key command or the
> toolbar icon. Which basically means that you would go back and forward depending
> on the bm manager being open or not. 

This is impossible if BM was the first thing to load in a window or tab, becuase
you have nothing to go back to. Safari doesn't toggle, and, in this "bookmarks
as web page" model I don't think it makes sense.

Comment 18

[louis bennett]

(In reply to comment #17)
> This is impossible if BM was the first thing to load in a window or tab, becuase
> you have nothing to go back to. Safari doesn't toggle, and, in this "bookmarks
> as web page" model I don't think it makes sense.

In that case, would it make sense to toggle to a blank page?

Comment 19

[Mike Pinkerton (not reading bugmail)]

-  BOOL            overContentArea = NSPointInRect(localPoint, [self contentRect]);
+  //BOOL            overContentArea = NSPointInRect(localPoint, [self
contentRect]);

just remove it. happens twice.

+  [self loadURL:@"about:bookmarks" referrer:nil activate:YES allowPopups:YES];

doubt this matters, but |allowPopups| should prolly be NO just for safety sake?

+
+#include <QuickTime/QuickTime.h>
+

does this get rid of warnings? why is it in here?

just a few things to check:
- multiple bookmark views in tabs don't get out of sync
- having bookmarks in the session history multiple times (bookmarks, page, page,
page, bookmarks, page, bookmarks, etc) doesn't leak or get out of sync.
- setting it as the startup url works
- do we want to add a radio button to have all new windows/tabs load with
bookmarks/history? or maybe explanatory text under the "home page" to say if
they want to use bookmarks to use "about:bookmarks"? not sure which is easier to
comprehend.

looking good otherwise, this will help streamline bits of the overlycomplex BWC

r=pink

Comment 20

[Simon Fraser [no longer active]]

> - multiple bookmark views in tabs don't get out of sync

Will be no different from > 1 window showing bookmarks, which works.

> - having bookmarks in the session history multiple times (bookmarks, page, page,
> page, bookmarks, page, bookmarks, etc) doesn't leak or get out of sync.

There is one bookmarksviewcontroller per tab, irrespective of how many times
it's in the history. All that's in the history is the url.

> - setting it as the startup url works

Check.

> - do we want to add a radio button to have all new windows/tabs load with
> bookmarks/history? or maybe explanatory text under the "home page" to say if
> they want to use bookmarks to use "about:bookmarks"? not sure which is easier to
> comprehend.

Perhaps, yes.

Comment 21

[Simon Fraser [no longer active]]

Checked in the patch with comments addressed. Please file separate bugs for any
regressions.