Closed Bug 119599 Opened 23 years ago Closed 22 years ago

Implement bookmark groups

Categories

(SeaMonkey :: Bookmarks & History, defect)

Product:
SeaMonkey
Component:
Bookmarks & History
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla1.0

People

(Reporter: hyatt, Assigned: hyatt)

References

Details

(Whiteboard: [ADT2])

Attachments

(2 files, 4 obsolete files)

Fix jag's concerns.
27.28 KB, patch
bugs
: review+
jag+mozilla
: superreview+
asa
: approval+
Details | Diff | Splinter Review
bookmark-group icons for modern and classic
1.29 KB, application/x-zip-compressed
Details 
Implement bookmark groups (a folder that acts like a leaf in the browser window
that, when clicked, loads a set of tabs).  In other words, this bug represents
the ability to bookmark a working set of tabs.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.8
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Adding keyword nsbeta1 to several bugs.
Keywords: nsbeta1
nsbeta1+ per Nav triage team
Keywords: nsbeta1 → nsbeta1+
cc kmurray.  who will implement this valuable feature?
Is there a spec for this?
->jag/1.0.  Is this on the landing plan?
Assignee: hyatt → jaggernaut
Status: ASSIGNED → NEW
Target Milestone: mozilla0.9.9 → mozilla1.0
Yeah. I took this as part of bug 103613, but I've added an explicit reference to
this now.
Blocks: 103613
--> me.
Assignee: jaggernaut → hyatt
Attached patch Patch for grps (obsolete) — — Splinter Review 

    
    

    
  
The prntf has been removed.  Pay no attenton to that.

Status: NEW → ASSIGNED

    
    

    
  
Comment on attachment 73761 [details] [diff] [review]
Patch for grps

sr=jag

        Attachment #73761 -
        Flags: superreview+

    
    

    
  
Comment on attachment 73761 [details] [diff] [review]
Patch for grps

sr=jag

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Better patch. (obsolete)
        — 
          — Splinter Review
      

    


  

        Attachment #73761 -
        Attachment is obsolete: true

    
    

    
  
Need r/sr again.


    
  
OS: Windows 2000 → All
Hardware: PC → All

    
    

    
  
Comment on attachment 73822 [details] [diff] [review]
Better patch.

Not all openItem calls have been patched and the if check in openItem lost its
curly bracket so it only applies to the next line (no idea why there's no
complaint about an extra closing curly bracket).

        Attachment #73822 -
        Flags: needs-work+

    
    

    
  


  

        Attachment #73822 -
        Attachment is obsolete: true

    
    

    
  
ADT3 per ADT triage. 

Evelyn to check if this is still a GO for MachV.
Whiteboard: [ADT3]

    
    

    
  
Comment on attachment 74142 [details] [diff] [review]
Fix jag's concerns.

>Index: themes/modern/communicator/bookmarks/bookmarks.css
>===================================================================
>RCS file: /cvsroot/mozilla/themes/modern/communicator/bookmarks/bookmarks.css,v
>retrieving revision 1.20
>diff -u -r1.20 bookmarks.css
>--- themes/modern/communicator/bookmarks/bookmarks.css	16 Jan 2002 03:01:17 -0000	1.20
>+++ themes/modern/communicator/bookmarks/bookmarks.css	14 Mar 2002 19:49:49 -0000
>@@ -63,3 +63,13 @@
> outlinerchildren:-moz-outliner-image(Name) {
>   padding-right: 2px;
> }
>+
>+outlinerchildren:-moz-outliner-twisty(group,hidetwisty) 
>+{
>+  list-style-image: none;
>+}
>+
>+outlinerchildren:-moz-outliner-image(Name, container, group) 
>+{
>+  list-style-image: url("chrome://communicator/skin/bookmarks/bookmark-group.gif");
>+}
>\ No newline at end of file

     ^^^^^^^^^^^^^^^^^^^

>Index: xpfe/components/bookmarks/resources/bookmarks.xml
>===================================================================
>RCS file: /cvsroot/mozilla/xpfe/components/bookmarks/resources/bookmarks.xml,v
>retrieving revision 1.32
>diff -u -r1.32 bookmarks.xml
>--- xpfe/components/bookmarks/resources/bookmarks.xml	12 Mar 2002 22:42:10 -0000	1.32
>+++ xpfe/components/bookmarks/resources/bookmarks.xml	14 Mar 2002 19:49:58 -0000
>@@ -1800,8 +1800,17 @@
>       <method name="openItem">
>       <parameter name="aEvent"/>
>       <parameter name="aInNewWindow"/>
>+      <parameter name="aOpenGroups"/>
>         <body><![CDATA[
>-          if (this.outlinerBoxObject.view.isContainer(this.outlinerBoxObject.selection.currentIndex)) {
>+          dump(this.outlinerBoxObject.selection.currentIndex);

Remove this dump.

>@@ -1824,20 +1833,37 @@

...

>+                if (!groupTarget) {
>+                  urlRes = this.rdf.GetResource(this.NC_NS + "URL");
>+                  urlValue = this.db.GetTarget(this.outlinerBuilder.getResourceAtIndex(k), urlRes, true).QueryInterface(Components.interfaces.nsIRDFLiteral).Value;
>+                  // Ignore "NC:" and empty urls
>+                  if (!urlValue || urlValue.substring(0,3) == "NC:") return;
>+                }
>+                else urlValue = "about:blank";

To stay in style you should put the body of this else on the next line.

>+                
>+                var w = openDialog (this._browserURL, "_blank", "chrome,all,dialog=no", urlValue);

Space before parenthesis after function name.

>+                if (groupTarget)
>+                  w.OpenBookmarkGroupFromResource(this.outlinerBuilder.getResourceAtIndex(k),
>+                                                  this.db, this.rdf);

...

>+              if (!urlValue || urlValue.substring(0,3) == "NC:") return;
>+            }
>+            else 
>+              urlValue = "about:blank";
>+              
>+            var w = openTopWin(urlValue);

Probably should do

  if (groupTarget)

>+            w.OpenBookmarkGroupFromResource(this.currentRes, this.db, this.rdf);
>           }
>           if (aEvent) 
>             aEvent.preventBubble();

sr=jag with these nits fixed.

        Attachment #74142 -
        Flags: superreview+

    
    

    
  
changing to adt2 per Evelyn.
Whiteboard: [ADT3] → [ADT2]

    
    

    
  


  
I really want to see this implemented (was actually implementing it myself as
an add on when I found this bug), to that end, I've attached the bookmark group
icons that you'll need. Sorry if you already have these, but I didnt see them
on lxr so I assumed you didn't have them.

    
    

    
  
I already have icons, but thanks.


    
  

        Attachment #74142 -
        Flags: approval+

    
    

    
  
Comment on attachment 74142 [details] [diff] [review]
Fix jag's concerns.

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

    
    

    
  
could someone attach an informal spec about how to use this feature by manually
editing their bookmarks file (until such a time as there's a UI for it)?

If so, I'll certainly be trying this code out :)

-matt

    
    

    
  
Add FOLDER_GROUP="true" after the id of a folder in the HTML.


    
    

    
  
It looks like hyatt forgot to check in the changes to utilityOverlay.js, you'll 
need those for bookmark groups to work properly in the sidebar, I'll check that 
in ASAP.

    
    

    
  
I don't see that bookmark groups will work in a subfolder of the personal
toolbar (obviously they will work on the personal toolbar itself).

    
    

    
  
*** Bug 132960 has been marked as a duplicate of this bug. ***

    
    

    
  


  
This will add a checkbox to the normal "File Bookmark" dialog when more than
one tab is open. Checking this will allow you to file the group of tabs as a
bookmark group.

    
    

    
  
Comment on attachment 76368 [details] [diff] [review]
Patch to hook up UI through "File Bookmark"

Of course, this patch is immaculate because jag wrote it, but I can always
nitpick it, right! ;)

+    var URL;

I know URL is an abbreviation and all, but does it really need to be all caps?

+var gOldNameValue = "";
+var gOldURLValue = "";
+function toggleGroup()

Can we put a new line between the vars and the function?

sr=hewitt

        Attachment #76368 -
        Flags: superreview+

    
    

    
  
Comment on attachment 76368 [details] [diff] [review]
Patch to hook up UI through "File Bookmark"

You might want to test for the existence of the tabbed browser in
addBookmarkAs().
You might want to ensure that addBookmark.js ends in a newline.
In theory you can nest your two new <row>s inside their own <rows> and hide
that instead of using a broadcaster.

    
    

    
  
Comment on attachment 76368 [details] [diff] [review]
Patch to hook up UI through "File Bookmark"

You might want to test for the existence of the tabbed browser in
addBookmarkAs().
You might want to ensure that addBookmark.js ends in a newline.
In theory you can nest your two new <row>s inside their own <rows> and hide
that instead of using a broadcaster.

    
    

    
  
I tried editing my bookmarks.html file like Hyatt noted, and I have a bookmark
group folder.. its a folder that is a link (with a new icon) with some bookmarks
listed.   Its on my personal toolbar.. so when I can click on it, it opens all
my links like it should..

also bug 131542 has a bunch of related group UI/feature bug links.

    
    

    
  
Hmmm, I thought I had replied to this. Must've hit the "please enter your
password again" screen or something.

Hewitt: nits fixed

Neil: I'll add that newline. I think putting those two rows in another row has
the same elegance as using a single-celled table to put a border around
something. As for checking for tabbrowser, we decided a while ago that since
tabbrowser's here to stay, there's no need to bother with these (for us)
unnecessary |if|s. If someone wants to take the source and rip tabbrowser out,
it's up to them to make sure to fix these sites.

    
    

    
  
Comment on attachment 76368 [details] [diff] [review]
Patch to hook up UI through "File Bookmark"

r=bryner

        Attachment #76368 -
        Flags: review+

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Fix nits. (obsolete)
        — 
          — Splinter Review
      

    


  

        Attachment #76368 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 76665 [details] [diff] [review]
Fix nits.

Transferring r= and sr=

        Attachment #76665 -
        Flags: superreview+

        Attachment #76665 -
        Flags: review+

    
    

    
  
Comment on attachment 76665 [details] [diff] [review]
Fix nits.

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

        Attachment #76665 -
        Flags: approval+

    
    

    
  
Comment on attachment 76665 [details] [diff] [review]
Fix nits.

Patch has been checked in.

        Attachment #76665 -
        Attachment is obsolete: true

    
    

    
  
Where were the qa builds for this?  I see all sorts of problems with it, and I
think this needs to come out and have someone in QA go over it, as it can cause
dataloss (open a bookmark group with tabs already open, and you lose all
previous tabs), as is not functioning as expected.

    
    

    
  
Not to mention that the Create In tree in the dialog is pretty much useless
without resizing.

    
    

    
  
looks like there is a new bug in the latest patch, file bookmark gets resized to
default after every single use.. used from Bookmarks -> File Bookmark.

it looks like SizeToFit() Doesn't take into account persisted dialog size after
use and resets it.  seen in build 4-1 and 4-2 on w2k.  I've already filed a
similar bug once see bug 123207.

    
    

    
  
I should note that this resizing to default size *only happens* with *2 or more
tabs* open, ie File As Group UI comes up.  see other bug for other kinda resizing.

    
    

    
  
Has this been reopened, or was it never closed?

    
    

    
  
Adding myself.

    
    

    
  
kerz: this is as designed. Given the decision between adding tabs or reusing
current ones we chose reusing current ones. Adding tabs would mean a user could
end up with a lot of tabs real quick, and reusing current ones is more like what
happens for normal bookmarks, which also replace the current page, not add a new
tab/window.

Dean, Dennis: the problem I tried to solve was that if the dialog is persisted
in the default size, and then brought up in the "file as group" version, the OK
and Cancel button are pushed partially outside the dialog.

I guess I could store the current width and height in e.g. addgroupheight and
addgroupwidth attributes on window and set height and width to those on dialog
open in the addGroup mode.

Note that for the normal case (just one tab open) nothing should have changed.

    
    

    
  
just filed bug 135270 re: sidebar not calling the right dialog to allow adding
groups.

re: comment#40 jason, yes i freaked out too when I saw that, but Jag is right,
the behavior is consistent, plus one can just hit 'Back' in each tab to see the
previous content(just like you would in the single window case) so no dataloss.

    
    

    
  
*** Bug 135316 has been marked as a duplicate of this bug. ***

    
    

    
  
I'm sure I said this before, but bookmark groups don't work in subfolders of
your personal toolbar folder.
Pressing Ctrl+T lots of times before adding a group is somewhat fun because the
resulting group has (only) one about:blank entry.
jag: is it possible to persist the height and width of the tree instead?

    
    

    
  
From comment 46

What happens though when you already have more tabs opened than you have in a
bookmark group.  Say you have 10 tabs opened on screen, but only 5 in the group.
 When you open the group only get those 5 but loose the other 5 tabs you had
opened and there's no way to get those back.  Already happened to me a few
times.  Maybe the group can re-use enough tabs tabs that are already opened, but
keep any tabs over that number untouched.

    
    

    
  
Right-clicking on a bookmark group in my personal toolbar brings up a menu which
includes the option "Open in New Window".  Using this option doesn't do what I
would expect; instead of opening a new window and populating it with the
appropriate tabs, it opens up the "Manage Bookmarks" window.

Actually, now that I play around with it, several of the right-click menu
options seem to have problems:

"Expand" seems to do nothing.
"New Folder..." seems to do nothing.  (Probably unrelated to bookmark groups,
though, since it does nothing anywhere in the personal toolbar...)
"Properties" brings up a properties dialog that seems like it could have more
useful information in it.  (A list of the URLs in the group.  Also, can bookmark
groups have a keyword like regular bookmarks?)

2002040403 W2K SP2, if it matters.

P.S. As I mentioned to a friend yesterday, "Bookmark groups have made my life
better."

    
    

    
  
Jamie, is not not been marked fixed, nor closed yet.  We have some UI issues to
iron out, but the feature work had been checked in.

Jason, maybe the solution would be for you to have an 'Ask me' if you wanna
override your tabs that are open. File and RFE bug for this, if you may.

Mike, I'm not sure that we want the context menu on the personal group folders
to do much?  Probably need to file a new bug to fix the context menu.  If you
had the open in new window option then you could hook that up to Jason's need to
be asked when the link is fired.

Jag, I may have had mis-interpreted what I'm trying to note as file group UI
dialog.. Let me explain a little more detail.. as I think you already understand
what I asked, just wanna write it out a little better.  Offer just a few ideas
with your comments for a fix.


Explaining in detail: 

If you have 1 tab/page in the browser and open file bookmark dialog the first
time, you then want to adjust the window to a new size you like based on the
root level bookmark view.   If I do adjust it and hit [OK] or [x] it will
persist its size upon reuse.  Now if I open a new tab so I have 2, I want to
file a new bookmark from either tab, I hit file dialog.., it opens to default
size.  (I think at this step it should of been persisted from the last instance)   

- so your saying that the buttons will be off the dialog because you're adding a
few lines at the top is the reason for choosing the default size here, so that
isn't garbled.  

Question 1) 

Is there a way to remove the same amount of 'create in folder' box space above
the buttons here while adding in the lines to the top at the same time?

Solution 1) 

If so, then size could be persisted globably.. and adjust as needed much like
other UI items in Mozilla.

Up most Problem with last Patch: 

If you close all the open tabs so you only have 1, the original 'non group
folder UI' dialog does persist its size you adjusted it to in step one, and does
not come up as the default size.

If you are into filing a lot of bookmarks.. and always have 2 tabs or more it
shows the 'Group Folder UI dialog' which happens to be the default size each and
every time even after you adjust it, not persisting the size you adjusted it
to.. this gets real annoying.

Solution 2)

- I believe you're saying this is probably possible to persist the dialog size
instead of reseting it everytime by adding height and width attributes.  This
would also be a good solution. 


So could you then create a seperate persisted size based upon the added UI and
checkbox dialog state, you are saving two sizes, 1 for 1 tab, and 1 for 2 or
more tabs.



Solution 3) 

I guess another solution maybe to move the [ok] and [cancel] buttons up to the
right side under the [new folder] and [default] buttons.

------------------------------

So with that, this is all I'd like to see changed for this bug to close, The
rest works fine for my use.  It doesn't matter how you decide to fix it, just
I'd like to see my bookmark folders because the root level is fairly long if I
need to file 1 bookmark.  I'd like it to persist its size everytime it opens no
matter if I have 1 tab or more so I dont *go crazy* having to resize the window
everytime I file a new bookmark so I can find the folder that I wanna create in. 

-------------------

Jamie, Functionality this works fine, *the real problem is usability, end user
satisfaction at this point.  :)

Every other problem we can easily move to put in other bugs either RFE or other
issues.

    
    

    
  
> "New Folder..." seems to do nothing.  (Probably unrelated to bookmark groups,
> though, since it does nothing anywhere in the personal toolbar...)

I thought it was "Open in New Window" which doesn't work in the personal toolbar?

    
  
Blocks: bm-group-tracker

    
    

    
  
From http://bugzilla.mozilla.org/show_bug.cgi?id=119599#c51

I'd like to see Expand on a bookmark-group do exactly the same as on any other
bookmark folder, i.e. Expand should open the folder so that one can open a single
link of the bookmark group. 

    
    

    
  
Reading the comments, I have not seen a person satisfied with or not freaked by
the tab overriding except the team that decided this behaviour.
Dataloss has already been pointed out, think to a person that has collected a
lot of information in many tabs and clicks to a multiple bookmark with two
elements...

The current behaviour is not consistent with a single bookmark.
Loading a single bookmark overrides the current tab but does not delete the
other tabs.
I think that for consistency, loading multiple bookmarks should override the
current tab with the first element and insert the others elements after the
first one. If the insertion is too difficult to implement within a short time,
other elements should be appended.

    
    

    
  
One suggestion is to check to see if the active window has other tabs open and
present a dialog that lets them choose to either

A)  Override the other tabs 
or
B)  Open the Group in a new window.

Also, I haven't downloaded the patch, but what happens when, say, the fifth
tab is what's active... does it take control of 1-5 or start with 5 and go
from there adding new tabs?

    
    

    
  
People who'll (want to) use this feature are likely to have more than one tab
open 90% of the time. They won't be amused by your warning dialog.

Comment #55 was dead on. Current behavior is unacceptable. It should either be
disabled or implemented correctly as proposed in comment #55:

"..for consistency, loading multiple bookmarks should override the
current tab with the first element and insert the others elements after the
first one. If the insertion is too difficult to implement within a short time,
other elements should be appended."

    
    

    
  
I believe a bookmark group should always open an entirely new window, for
consistency, no matter whether there are multiple tabs open in any current
navigator window or not. Consistency with other bookmarks makes no sense because
a bookmark group simply isn't similar to an ordinary bookmark.

    
    

    
  
I disagree. The whole reason to use tabs is because you don't want to open all
those new windows.

I have a nearly empty local page set as my home. 70% of the time I start mozilla
though the first thing I do is click _a lot_ of bookmarks to open in that first
window and then subsequent tabs. Bookmark groups would simplify this process a
lot. With bookmark groups openening in a new window I'd be killing off the
original window 70% of the time mere seconds after I'd opened it.

I too think that comment 55 is both the most logical and most useful suggestion. 
And if people for some reason really find that behaviour unworkable 'yet another
preference' could always be added to change it... :)

    
    

    
  
pierre I do like that idea, I guess it should be appended to the tabs you
already have open, with then focus given to the first, or one can make (add) a
pref to tab-browing panel for this.. 

Jason, did you open a new bug about the Dataloss concern?  I guess my thought is
if you know you are going to click on it you ought be told what is going to
happen once you do.  If you already know what happens, then the person who does
this, you can go back on each tab without actually loosing information per se. 
 A bug ought to be filed as we need a way to address that seperately I think.  

Currently the icon consists of 3 bookmarks, grouped together, it doesn't seem
odd to me.  Or out of place.  

Jag, comments?

    
    

    
  
Guys, I Just decided, maybe I should file a new bug.. so I filed one for this
resizing problem bug 136835.

    
    

    
  
Any reason why this hasn't been closed?
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED

    
    

    
  
There *should* be an "Open in new tab" option in the bookmark context menu.  I
am constantly having to go File->New->Tab and the select a bookmark.  I'm sure
I'm not the only one.

This would also resolve the silly argument over whether opening bookmark folders
should use currently open tags.  "Open" would do this, "Open in new window"
opens a new set of tabs in a window, and "Open in new tabs" would use the
existing window with new tabs.

There, everybody is satisfied.

    
    

    
  
re: comment #62 blake's will be done :-)

VERIFIED Fixed on all platforms.

bug 134800 (culled from comment #50) should not be ignored however.
Status: RESOLVED → VERIFIED

    
    

    
  
One thing I really wanted to see in this was mentioned in dup bug 135316; that
being the ability to easily send the bookmark to yourself via email or to disk
(without having to root for the bookmarks.html and save that entire file). 

The example was that one would be doing research at work and want to take it home
easily, so you bookmark the group of open tabs and email it to yourself. You go
home and voila! your work is reinstated on your home pc.
Product: Browser → Seamonkey

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: