382896 - tooltip for folders shows internal location

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Jo Hermans]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070601 Minefield/3.0a5pre

When you hover over a link on the bookmarks toolbar, you see a tooltip with the name (should be a description, see bug 256065) and the location. But when you hover over a folder, you also see a tooltip. The second line contains the location, but that shows the internal URI. For instance, I have a folder that shows "place:folder=51&group=3" as the address.

What's the point in displaying that information ? It's not useful for a user, and it's not shown anywhere else (not in the properties for example, the location-field is hidden for folders).

It's probably a regression due to the Places changes. It's not present in FF 2.0.0.4, which doesn't show a tooltip for folders at all. Either don't show a tooltip at all, or remove the second line.

Comment 1

[Adam Guthrie]

I disagree with this being only a minor issue. The same thing happens when you hover the default "Latest Feeds" RSS feed that comes by default in all new installs. This is probably something we want to fix for Firefox 3.

Comment 2

[(not reading, please use [e-mail] instead)]

egads, adam, you are right.  in fact, I was just in this code, see http://lxr.mozilla.org/seamonkey/source/browser/base/content/browser-places.js#337 and bug #379591.

for folders, we may still want to show tooltips, especially if the folder name is too long and becomes cropped, with ellipses. 

or we might want to keep fx parity.  either way, we don't want to show uris.

Comment 3

[(not reading, please use [e-mail] instead)]

patch in hand to restore fx2 parity.  as for showing the tooltip when the folder is cropped, I'll log a new bug about that and seek UI approval.  ideally, we'd only show the tooltip for the folder if the name was cropped (like we do with trees)

here comes a patch.

Comment 4

[(not reading, please use [e-mail] instead)]

Attachment: patch

credit to mano's foresight:  for bug #379591 he suggested we pass in the node, and not the url as an attribute, which makes this super easy to fix.

Comment 6

[Asaf Romano (gone)]

I would argue that the correct check here is !nodeIsURI.

Comment 7

[(not reading, please use [e-mail] instead)]

> I would argue that the correct check here is !nodeIsURI.

I'll fix the patch to use !nodeIsURI and seek another review.

Comment 8

[(not reading, please use [e-mail] instead)]

Attachment: revised patch, per mano

Comment 9

[Asaf Romano (gone)]

Comment on attachment 268032 [details]
revised patch, per mano

nit: trailing whitespace.

r=mano.

Comment 10

[(not reading, please use [e-mail] instead)]

fixed.

Checking in browser-places.js;
/cvsroot/mozilla/browser/base/content/browser-places.js,v  <--  browser-places.j
s
new revision: 1.35; previous revision: 1.34
done

Comment 11

[Jo Hermans]

verified as fixed
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a6pre) Gecko/20070613 Minefield/3.0a6pre

Comment 12

[Jeff Walden [:Waldo]]

This should be a Litmus test, probably folded into an existing test.

Comment 13

[Adam Guthrie]

http://litmus.mozilla.org/show_test.cgi?id=4422

Comment 14

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv