360464 - Dock menu should fall back on URLs if bookmark titles aren't populated

Status: RESOLVED FIXED

(Camino Graveyard :: Toolbars & Menus, enhancement)

Description

[adam cheasley]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.7) Gecko/20060911 Camino/1.0.3
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.7) Gecko/20060911 Camino/1.0.3

If I right click on the icon in the dock there should be a list of recently visited URLs, but at present they are all blank.  The URLs still seem to work but I can't tell what I'm clicking on.

Reproducible: Always




My mac is very new (about 2 weeks old) and I've hardly changed any of the default settings to the OS yet.

Comment 1

[Chris Lawson (gone)]

The default dock menu grouping in Camino is the "Top Ten" in the bookmarks manager.

Are you saying that there's a dock menu with active menu items, but all the menu items are blank? Or is there a different problem? Either way, please take a screen shot with Command-Shift-4 and attach the resulting file to this bug.

cl

Comment 2

[Stuart Morgan]

Also, please see if this is reproducible with 1.1a1

Comment 3

[adam cheasley]

Attachment: screen shot of the blank top ten in the dock

Comment 4

[adam cheasley]

(In reply to comment #1)
> The default dock menu grouping in Camino is the "Top Ten" in the bookmarks
> manager.
> 
> Are you saying that there's a dock menu with active menu items, but all the
> menu items are blank?

Yes.  I believe when I first installed it they were working, but for the past week or so they have just been blank.

Comment 5

[froodian (Ian Leue)]

Please do try comment 2.  Also, do you still have your bookmarks in general?  If so, if you go to bookmarks, is the top ten list correctly populated there?  If so, if you control-click on the top ten list and choose "use as dock menu", does it fix the problem?

Comment 6

[adam cheasley]

Attachment: same problem with 1.1a on a different mac

This is the problem replicated on 1.1a on my mac at work which is a G4 powerbook 1.67 GHz running OSX 10.4.8

Comment 7

[adam cheasley]

(In reply to comment #5)
> Please do try comment 2.  Also, do you still have your bookmarks in general? 
> If so, if you go to bookmarks, is the top ten list correctly populated there? 
> If so, if you control-click on the top ten list and choose "use as dock menu",
> does it fix the problem?
> 
I tried this but doesn't seem to fix the problem.  Also on the latest screen shot you can see that two of the URLs have titles and the rest are blank as before.

Comment 8

[Chris Lawson (gone)]

If it's OK with you, would you mind attaching your specific bookmarks.plist file to this bug? If there's stuff in there you don't want posted, please make a backup copy of the file, then delete any sensitive bookmarks *from the original* and post that.

Once you've posted the original, you can delete it. Rename the backup to "bookmarks.plist" while Camino is NOT RUNNING and you should have everything back to the way it was.

cl

Comment 9

[adam cheasley]

I've just been looking through my bookmarks and I've figured out what the problem is.  
I now feel very very stupid and I apologize for wasting peoples time.
The problem is caused by the way I like the bookmark bar set up.  I prefer to have just the icon (favicon) associated with the site shown and not a title.  So when I put a URL in my bookmarks bar I goto get info and delete the title so that my bar is just icons.  The menu from the dock must be populated by the titles and not the actual URLs.  This makes sense to me now.  That's why Oomph was showing up in the second screen shot because it doesn't have a favicon.
Sorry again.

Comment 10

[Chris Lawson (gone)]

(In reply to comment #9)
> The menu from the dock must be populated by the
> titles and not the actual URLs.

See, I figured we had fallback code that was smart enough to handle that case. Obviously we don't. ;)

We probably should. In cases like this, we could simply put the icon in the menu, or we could fall back on the URL if the title is blank, or some combination of the two. Thoughts?

cl

Comment 11

[adam cheasley]

It would be nice if it showed the icon then the title.  Then if there was no title you would still be left with the icon, although I'm sure it would be easier to just fall back on the URLs.

Comment 12

[Chris Lawson (gone)]

Confirming. We need to discuss how we're going to order the fallback per comment 10.

cl

Comment 13

[Chris Peterson [:cpeterson]]

Attachment: bookmark-menu-v1.patch

With this patch, menu item titles for both the Dock menu and Camino's Bookmarks menu will be:

1. the bookmark Title
2. or the bookmark Description
3. or the bookmark Location URI
4. or a zero-length string

AFAIK, Dock menus do not support icons:
http://www.cocoabuilder.com/archive/cocoa/98517-icons-in-dock-menu.html

Comment 14

[Chris Lawson (gone)]

Comment on attachment 485475 [details] [diff] [review]
bookmark-menu-v1.patch

A couple comments and nits on this:

1) Nit: the patch uses a mix of |NSFoo* bar| and |NSFoo *bar|. Pick one or the other and stick with it, please :) (Yes, the file is somewhat inconsistent, too. For the record, I prefer NSFoo*, but that's just me.)

2) As you note in your comment, how on earth are we going to get a bookmark with no title, no description, and no URL? That seems like a rare enough case that we might want to have an obvious placeholder rather than an empty title, maybe something like "<EMPTY BOOKMARK>"? Others can chime in on whether this is a good idea or not.

3) Smokey mentioned on IRC, and I agree with him, that description might be something we'd want to ignore entirely. Glancing through my reasonably large bookmarks file, I don't see a single description anywhere. Do we have any idea if people actually use this?

I'm basically unavailable for reviews for the next two weeks, starting tomorrow morning. I'm going to cancel this review request for now, since there are a couple things that need discussion, and if there's a new patch before 07 November, someone else will need to review it.

Thanks for taking this, Chris!

Comment 15

[Chris Peterson [:cpeterson]]

Attachment: bookmark-menu-v2.patch

Incorporate cl's feedback:
1. Fixed pointers.
2. For very empty bookmarks, return the localized string used for empty tab titles ("UntitledPageTitle" = "Untitled").
3. Ignore bookmark Description. I have never used bookmark descriptions either.

Comment 16

[Stuart Morgan]

Comment on attachment 485513 [details] [diff] [review]
bookmark-menu-v2.patch

>+  // Return "Untitled" title like we do for empty tabs.
>+  return NSLocalizedString(@"UntitledPageTitle", nil);

I'm not comfortable assuming that these will be identical in all languages. Since it should never come up in normal use I don't think we need to go out of our way; let's just return an empty string.

Comment 17

[Stuart Morgan]

(But sr=smorgan with that change)

Comment 18

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

cl and I think this really needs use a specific-to-this-case localized string rather than an empty string.  Returning "" is just confusing to users and is asking for bug reports when they end up with empty bookmarks in their Dock menu's bookmark source.  It's trivial to do this right (just as it's pretty easy to end up with a bookmark in this state, either on purpose or by accident).

Also, Stuart, did you r or sr the patch (or both)?  Attachments table shows r, comment 17 says sr. ;)

Comment 19

[Stuart Morgan]

Comment on attachment 485513 [details] [diff] [review]
bookmark-menu-v2.patch

Oops, I thought it was an sr request. Now it is!

> Returning "" is just confusing to users

But it will be less confusing when has a name, but still doesn't open anything?

> It's trivial to do this right

I don't really see why giving a broken a bookmark a name is much more right.

> just as it's pretty easy to end up with a bookmark in this
> state, either on purpose or by accident

Seriously? I've never accidentally deleted both the title *and* URL of one of my bookmarks, and I'm not clear on how that would happen. As for on purpose... if someone deliberately guts a bookmark to the point where it's completely empty, why would they be surprised that it's empty in menus?

Comment 20

[Chris Lawson (gone)]

(In reply to comment #19)
> Seriously? I've never accidentally deleted both the title *and* URL of one of
> my bookmarks, and I'm not clear on how that would happen. As for on purpose...
> if someone deliberately guts a bookmark to the point where it's completely
> empty, why would they be surprised that it's empty in menus?

That makes me wonder if maybe we can just skip putting those bookmarks in the menu entirely. Seems a lot more logical, since otherwise we end up with non-functional menu items (that either have a title or don't).

Comment 21

[Stuart Morgan]

Or, users who gut a bookmark and then aren't happy about having it could just go delete it.

Our goal here isn't to prevent users in contrived cases from deliberately confusing themselves. Is it also worth extra code complexity to specially handle cases where the name is a URL, but not the actual bookmark URL? Could be confusing. Or what if a user switches the titles of two bookmarks? Or puts garbage in the URL--or just a made up domain name?

This bug is about fixing an annoying side-effect of a real-world use case: removing the title of bookmarks in the bookmark bar. Let's just fix that, and not a bunch of made-up edge cases.

Comment 22

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

(In reply to comment #19)
> if someone deliberately guts a bookmark to the point where it's completely
> empty, why would they be surprised that it's empty in menus?

They did it at some point and later forgot about it, and now they have no idea what's going on ;)  Anyway, that's fine.

Chris, no need to post a new patch; I'll just switch back in the 'return @"";' line from the original patch when I go land this later.

Comment 23

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

http://hg.mozilla.org/camino/rev/a4a85bb54d64 with the empty string instead of a localized string in the final fallback case :)

Comment 24

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

And http://hg.mozilla.org/camino/rev/0558b6835a0a because I forgot the comment had been modified, too :(