Last Comment Bug 244116 - Unable to delete bookmark folder that is flagged as Dock Menu + cannot remove from Dock
: Unable to delete bookmark folder that is flagged as Dock Menu + cannot remove...
Status: VERIFIED FIXED
: verified1.8.1
Product: Camino Graveyard
Classification: Graveyard
Component: Bookmarks (show other bugs)
: unspecified
: PowerPC Mac OS X
-- normal (vote)
: Camino1.5
Assigned To: froodian (Ian Leue)
:
:
Mentors:
: 314629 319297 327995 357353 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-05-19 19:08 PDT by derek fong
Modified: 2006-10-20 06:48 PDT (History)
10 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch to allow to unceck "Dock menu" as well as deleting the dock menu folder (6.67 KB, patch)
2005-05-06 07:15 PDT, Ulrik Sverdrup
englabenny: review+
Details | Diff | Splinter Review
Same patch updated to don't conflict with current source (5.89 KB, patch)
2005-05-31 15:58 PDT, Ulrik Sverdrup
sfraser_bugs: review-
Details | Diff | Splinter Review
Patch to allow unchecking and transferring the dock flag, without the allow-delete changes (4.92 KB, patch)
2005-05-31 16:00 PDT, Ulrik Sverdrup
sfraser_bugs: review-
Details | Diff | Splinter Review
Patch to allow checking/unchecking of dock menu checkbox for bookmark folders (6.85 KB, patch)
2005-08-29 19:13 PDT, Ulrik Sverdrup
stuart.morgan+bugzilla: review-
Details | Diff | Splinter Review
Increment to do the full fix; allow deletion of dock menu folders (1.26 KB, patch)
2005-08-29 19:29 PDT, Ulrik Sverdrup
no flags Details | Diff | Splinter Review
Increment to do the full fix; allow deletion of dock menu folders (fixed edition) (1.26 KB, patch)
2005-08-29 19:38 PDT, Ulrik Sverdrup
no flags Details | Diff | Splinter Review
Brings Ulrik's patches up to date (8.98 KB, patch)
2006-07-04 01:43 PDT, froodian (Ian Leue)
stuart.morgan+bugzilla: review+
Details | Diff | Splinter Review
Gets rid of defaultDockMenuFolder (7.04 KB, patch)
2006-07-21 06:15 PDT, froodian (Ian Leue)
stuart.morgan+bugzilla: review+
mikepinkerton: superreview+
Details | Diff | Splinter Review

Description User image derek fong 2004-05-19 19:08:38 PDT
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7) Gecko/20040517 Camino/0.8b
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7) Gecko/20040517 Camino/0.8b

There appears to be a problem manipulating bookmark folders that are used in the
Dock.  Specifically, you are unable to "unmark" a bookmark folder as being in
the Dock and you are unable to actually delete the bookmark folder.

Reproducible: Always
Steps to Reproduce:
1. Open the Bookmarks pane.
2. Create a new bookmark folder.
3. Get Info on the new bookmark folder you just created.
4. Check the Dock Menu checkbox.

Immediately, the "Dock Menu" item is disabled.  There is no way to remove this
folder from the Dock.

Additionally, to reproduce the second part of this bug:

5. Back in the Bookmarks pane, right-click the bookmark folder you created, and
select Delete.

Actual Results:  
The bookmark folder remains.  Sometimes the bookmark folder disappears (i.e., if
you created a bookmark folder for use in the Dock in the Toolbar), but upon
opening a new window, the bookmark folder reappears and there is no way to
remove it.

Expected Results:  
The bookmark folder should be deleted when you choose to delete it, and you
should be able to toggle the Dock status on and off at will.
Comment 1 User image Stuart Morgan 2004-05-19 19:43:52 PDT
Yes, the current behavior is definitely bad.

There's a work-around (select a different folder as the dock folder, which will
unmark the old dock folder), but it's not at all intuitive. We should allow no
folder to be selected, or default to something like the Toolbar that should
always be there when a folder is un-checked (which do we do for a new profile?).
Comment 2 User image Ulrik Sverdrup 2005-05-06 06:29:00 PDT
I am trying to fix this one. I'll let BookmarkManager have a method
+defaultDockMenuFolder, and always fall back on that one. Sounds reasonable?
Comment 3 User image Mike Pinkerton (not reading bugmail) 2005-05-06 06:32:33 PDT
should we prompt the user first and let them know what's going on and what we'll
be setting the dock menu to?
Comment 4 User image Jasper 2005-05-06 06:37:43 PDT
Sounds like a good plan to me, default dock menu on a fresh install is the
topten bm folder so you could use that I guess.
Comment 5 User image Ulrik Sverdrup 2005-05-06 07:15:51 PDT
Created attachment 182783 [details] [diff] [review]
Patch to allow to unceck "Dock menu" as well as deleting the dock menu folder

Modifies behavior to not guard the dock menu folder and lets it be deleted and
unset. Reverts to default dock menu folder (Given by a BookmarksManager method)
when it's needed.

Also fixes a bug in connection to this: When copying the dock menu bookmark
folder, the dock menu folder flag was transferred.

Does not give a warning upon deleting the dock menu folder
Comment 6 User image Ulrik Sverdrup 2005-05-06 07:28:54 PDT
(In reply to comment #5)
The small bug I mentioned is Bug 293147
Comment 7 User image Ulrik Sverdrup 2005-05-31 15:58:12 PDT
Created attachment 184962 [details] [diff] [review]
Same patch updated to don't conflict with current source
Comment 8 User image Ulrik Sverdrup 2005-05-31 16:00:26 PDT
Created attachment 184963 [details] [diff] [review]
Patch to allow unchecking and transferring the dock flag, without the allow-delete changes

This patch lets the "dock menu" checkbox always be enabled, and lets the user
uncheck the current dock menu (camino falls back on a default dock menu,
currently top ten, which can't be unchecked (no info box on that one))

Also, you can check "dock menu" for any folder, and it will be set and the
previous one unset.
Comment 9 User image Josh Aas 2005-07-28 13:17:55 PDT
Comment on attachment 184962 [details] [diff] [review]
Same patch updated to don't conflict with current source

These are cool patches, but I'm not going to get to reviewing them soon so
asking smfr for review.
Comment 10 User image Simon Fraser 2005-07-28 13:24:55 PDT
Comment on attachment 184962 [details] [diff] [review]
Same patch updated to don't conflict with current source

Patch is stale copyWithZone: changed a while back.
Comment 11 User image Simon Fraser 2005-07-28 13:25:46 PDT
Comment on attachment 184963 [details] [diff] [review]
Patch to allow unchecking and transferring the dock flag, without the allow-delete changes

Stale patch. And do you wan this one or the last one or both? Don't make
overlapping patches.
Comment 12 User image Ulrik Sverdrup 2005-08-02 19:42:10 PDT
Sorry, I didn't understand in which form the patch was wanted - with the dock
flag fix or not. Now that the dock flag is checked in in another bug, that's
solved. 

I'm sorry but I can't build camino right now, and can't work at it for a short
while more. I'll come back to this bug in two-three weeks I hope.
Comment 13 User image Ulrik Sverdrup 2005-08-02 19:46:16 PDT
Comment on attachment 184962 [details] [diff] [review]
Same patch updated to don't conflict with current source

Obsoleting this patch. Includes dock flag changes already checked in
anotherplace.
Comment 14 User image Ulrik Sverdrup 2005-08-29 19:13:42 PDT
Created attachment 194267 [details] [diff] [review]
Patch to allow checking/unchecking of dock menu checkbox for bookmark folders

A patch from current source

Addresses: Allows the user to delselect the enabled "dock menu" checkbox for
bookmark folders. Handles fallback to default folder (top10). Handles updating
of checkbox if the dock menu changes while it is displayed.

Handles a minor thing outside it's scope: sets mDirty for the dock menu after
setBookmarksFolder: is called on it.

The patch includes no changes to allow deletion of the dock menu folder
Comment 15 User image Ulrik Sverdrup 2005-08-29 19:29:41 PDT
Created attachment 194269 [details] [diff] [review]
Increment to do the full fix; allow deletion of dock menu folders

With the above patch in place, the step to allow and handle deletion of the
dock menu folder is trivial. This is an increment of the previous patch.

The following patch changes one line and adds two. isDockMenu does not make a
bookmarks folder special anymore.
Comment 16 User image Ulrik Sverdrup 2005-08-29 19:38:11 PDT
Created attachment 194272 [details] [diff] [review]
Increment to do the full fix; allow deletion of dock menu folders (fixed edition)

The previous patch was reversed by mistake. This one is diffed in the right
direction.

 I'm sorry, and yes, I'm going to sleep now to not make more silly mistakes.
Comment 17 User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2005-11-01 19:29:06 PST
*** Bug 314629 has been marked as a duplicate of this bug. ***
Comment 18 User image Stuart Morgan 2005-12-06 08:12:22 PST
*** Bug 319297 has been marked as a duplicate of this bug. ***
Comment 19 User image Stuart Morgan 2006-02-23 18:58:56 PST
*** Bug 327995 has been marked as a duplicate of this bug. ***
Comment 20 User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2006-02-23 21:54:15 PST
See also bug 314628 for improving the "Dock menu" experience once this patch gets in....
Comment 21 User image Stuart Morgan 2006-02-26 16:43:20 PST
Comment on attachment 194267 [details] [diff] [review]
Patch to allow checking/unchecking of dock menu checkbox for bookmark folders

Unfortunately, this patch has suffered some bitrot.  If someone has time to clean it up to apply to the current tree, I'll review it soon.
Comment 22 User image Mike Pinkerton (not reading bugmail) 2006-03-14 08:42:09 PST
so what does the dock menu get reset to after you delete it? don't you have to manually set it back to something else?
Comment 23 User image Samuel Sidler (old account; do not CC) 2006-04-30 00:39:36 PDT
Comment on attachment 194272 [details] [diff] [review]
Increment to do the full fix; allow deletion of dock menu folders (fixed edition)

Clearing review request for bitrotten patch.
Comment 24 User image Samuel Sidler (old account; do not CC) 2006-04-30 00:40:54 PDT
Ulrik, can you create a new patch based on the current tree? We'd love to get this in for 1.1. :)
Comment 25 User image erkan 2006-07-03 23:32:30 PDT
This was a problem 2 years ago and  still a problem in the new release? That sucks..
Comment 26 User image froodian (Ian Leue) 2006-07-04 01:43:06 PDT
Created attachment 228030 [details] [diff] [review]
Brings Ulrik's patches up to date

This applies cleanly and works as expected.  I'm not intimately acquainted with the code, but I did apply it all by hand and read it as I did, and I think I have a pretty decent understanding of what's going on, so I'm gonna take responsibility for it from now on.  Ulrik made this as two patches, to land incrementally.  Since this isn't really that big a landing and we don't do that very much, I rolled it up into one patch.
Comment 27 User image Stuart Morgan 2006-07-20 22:52:51 PDT
Comment on attachment 228030 [details] [diff] [review]
Brings Ulrik's patches up to date

r=me. Looks good, and provides a much better user experience than the current system.

pink, in answer to your question: there's no need to explicitly mark the fallback (top-10) as the dock menu because there's no way to get info on a collection, so from a user perspective there's no difference between having no explicit dock menu and having top-10 explicitly marked as the dock menu. Code-wise, all access to the dock menu is through a function that transparently substitutes in the fallback if there isn't an explicit one.
Comment 28 User image Stuart Morgan 2006-07-20 23:02:59 PDT
Comment on attachment 228030 [details] [diff] [review]
Brings Ulrik's patches up to date

>+-(BookmarkFolder *) defaultDockMenuFolder;
...
>-    return [self top10Folder];
>+    return [self defaultDockMenuFolder];
>+}
>+
>+-(BookmarkFolder *) defaultDockMenuFolder {
>+  return [self top10Folder];

Actually... I had decided against requesting this change, but thinking about it more I really can't come up with any reason someone would need to be able to find out what the default dock menu folder is; only dockMenuFolder should know or care about the fallback.

So r=me with the above section of changes removed.
Comment 29 User image froodian (Ian Leue) 2006-07-21 06:15:29 PDT
Created attachment 230121 [details] [diff] [review]
Gets rid of defaultDockMenuFolder
Comment 30 User image Mike Pinkerton (not reading bugmail) 2006-07-21 08:29:39 PDT
Comment on attachment 230121 [details] [diff] [review]
Gets rid of defaultDockMenuFolder

sr=pink
Comment 31 User image Nick Kreeger 2006-07-22 21:19:06 PDT
Fixed trunk and branch.
Comment 32 User image HG 2006-10-20 06:48:10 PDT
*** Bug 357353 has been marked as a duplicate of this bug. ***

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