Closed
Bug 119599
Opened 23 years ago
Closed 22 years ago
Implement bookmark groups
Categories
(SeaMonkey :: Bookmarks & History, defect)
SeaMonkey
Bookmarks & History
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla1.0
People
(Reporter: hyatt, Assigned: hyatt)
References
Details
(Whiteboard: [ADT2])
Attachments
(2 files, 4 obsolete files)
27.28 KB,
patch
|
bugs
:
review+
jag+mozilla
:
superreview+
asa
:
approval+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Updated•23 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.8
Assignee | ||
Updated•23 years ago
|
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Comment 3•23 years ago
|
||
cc kmurray. who will implement this valuable feature?
Comment 4•23 years ago
|
||
Is there a spec for this?
Comment 5•22 years ago
|
||
->jag/1.0. Is this on the landing plan?
Assignee: hyatt → jaggernaut
Status: ASSIGNED → NEW
Target Milestone: mozilla0.9.9 → mozilla1.0
Comment 6•22 years ago
|
||
Yeah. I took this as part of bug 103613, but I've added an explicit reference to this now.
Blocks: 103613
Assignee | ||
Comment 8•22 years ago
|
||
Assignee | ||
Comment 9•22 years ago
|
||
The prntf has been removed. Pay no attenton to that.
Status: NEW → ASSIGNED
Comment 10•22 years ago
|
||
Comment on attachment 73761 [details] [diff] [review] Patch for grps sr=jag
Attachment #73761 -
Flags: superreview+
Comment 11•22 years ago
|
||
Comment on attachment 73761 [details] [diff] [review] Patch for grps sr=jag
Comment 12•22 years ago
|
||
Comment on attachment 73761 [details] [diff] [review] Patch for grps r=ben@netscape.com
Attachment #73761 -
Flags: review+
Assignee | ||
Comment 13•22 years ago
|
||
Attachment #73761 -
Attachment is obsolete: true
Assignee | ||
Comment 14•22 years ago
|
||
Need r/sr again.
Updated•22 years ago
|
OS: Windows 2000 → All
Hardware: PC → All
Comment 15•22 years ago
|
||
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+
Assignee | ||
Comment 16•22 years ago
|
||
Attachment #73822 -
Attachment is obsolete: true
Comment 17•22 years ago
|
||
ADT3 per ADT triage. Evelyn to check if this is still a GO for MachV.
Whiteboard: [ADT3]
Comment 18•22 years ago
|
||
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+
Comment 20•22 years ago
|
||
Comment on attachment 74142 [details] [diff] [review] Fix jag's concerns. r=ben@netscape.com
Attachment #74142 -
Flags: review+
Comment 21•22 years ago
|
||
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.
Assignee | ||
Comment 22•22 years ago
|
||
I already have icons, but thanks.
Updated•22 years ago
|
Attachment #74142 -
Flags: approval+
Comment 23•22 years ago
|
||
Comment on attachment 74142 [details] [diff] [review] Fix jag's concerns. a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Comment 24•22 years ago
|
||
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
Assignee | ||
Comment 25•22 years ago
|
||
Add FOLDER_GROUP="true" after the id of a folder in the HTML.
Comment 26•22 years ago
|
||
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.
Comment 27•22 years ago
|
||
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).
Comment 28•22 years ago
|
||
*** Bug 132960 has been marked as a duplicate of this bug. ***
Comment 29•22 years ago
|
||
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 30•22 years ago
|
||
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 31•22 years ago
|
||
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 32•22 years ago
|
||
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 33•22 years ago
|
||
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.
Comment 34•22 years ago
|
||
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 35•22 years ago
|
||
Comment on attachment 76368 [details] [diff] [review] Patch to hook up UI through "File Bookmark" r=bryner
Attachment #76368 -
Flags: review+
Comment 36•22 years ago
|
||
Attachment #76368 -
Attachment is obsolete: true
Comment 37•22 years ago
|
||
Comment on attachment 76665 [details] [diff] [review] Fix nits. Transferring r= and sr=
Attachment #76665 -
Flags: superreview+
Attachment #76665 -
Flags: review+
Comment 38•22 years ago
|
||
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 39•22 years ago
|
||
Comment on attachment 76665 [details] [diff] [review] Fix nits. Patch has been checked in.
Attachment #76665 -
Attachment is obsolete: true
Comment 40•22 years ago
|
||
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.
Comment 41•22 years ago
|
||
Not to mention that the Create In tree in the dialog is pretty much useless without resizing.
Comment 42•22 years ago
|
||
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.
Comment 43•22 years ago
|
||
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.
Comment 44•22 years ago
|
||
Has this been reopened, or was it never closed?
Comment 45•22 years ago
|
||
Adding myself.
Comment 46•22 years ago
|
||
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.
Comment 47•22 years ago
|
||
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.
Comment 48•22 years ago
|
||
*** Bug 135316 has been marked as a duplicate of this bug. ***
Comment 49•22 years ago
|
||
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?
Comment 50•22 years ago
|
||
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.
Comment 51•22 years ago
|
||
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."
Comment 52•22 years ago
|
||
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.
Comment 53•22 years ago
|
||
> "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
Comment 54•22 years ago
|
||
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.
Comment 55•22 years ago
|
||
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.
Comment 56•22 years ago
|
||
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?
Comment 57•22 years ago
|
||
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."
Comment 58•22 years ago
|
||
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.
Comment 59•22 years ago
|
||
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... :)
Comment 60•22 years ago
|
||
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?
Comment 61•22 years ago
|
||
Guys, I Just decided, maybe I should file a new bug.. so I filed one for this resizing problem bug 136835.
Comment 62•22 years ago
|
||
Any reason why this hasn't been closed?
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 63•22 years ago
|
||
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.
Comment 64•22 years ago
|
||
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
Comment 65•22 years ago
|
||
shouldnt http://komodo.mozilla.org/planning/branches.cgi be updated?
Comment 66•22 years ago
|
||
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.
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•