Closed Bug 237592 Opened 16 years ago Closed 11 years ago

Bookmarks/RSS items should always show tooltip when hovering with mouse

Categories

(Firefox :: Bookmarks & History, enhancement, P3)

enhancement

Tracking

()

VERIFIED FIXED
Firefox 3.1b2

People

(Reporter: djst, Assigned: Gabri)

References

(Blocks 1 open bug)

Details

(Keywords: fixed-seamonkey1.1a, fixed1.8.1, Whiteboard: [FF has patch][SM fixed/verified-seamonkey1.1a][no l10n impact])

Attachments

(7 files, 23 obsolete files)

16.40 KB, patch
sgautherie
: review+
sgautherie
: superreview+
Details | Diff | Splinter Review
16.45 KB, patch
Details | Diff | Splinter Review
13.40 KB, image/png
Details
48.46 KB, image/png
Details
17.11 KB, image/png
Details
3.62 KB, patch
Gavin
: review+
Details | Diff | Splinter Review
4.76 KB, patch
Details | Diff | Splinter Review
If you hover the mouse over a bookmark on the Bookmarks Toolbar, you get a
tooltip showing the full title and its URI. However, if you hover the mouse over
a bookmark inside a folder, or from the Bookmarks menu, you don't see the tooltip.

It should always be possible to see the tooltip, as in IE. It's faster than
right-clicking and selecting Properties just to verify the URI.
Should this be all the time, or only if you don't have the status bar visible? 
Currently, the status bar will also show you the URL (although this is sometimes
blocked if your bookmarks menu is tall enough)
The problem with the status bar is it's not obvious if you don't already know
about it.  Not to mention that bouncing your eyes from the bookmark menu at the
top of the screen to the status bar at the bottom as you mouse over each entry
is not very user friendly.

The tooltip should also contain the bookmark's description (if any), requested
in related bug 135962.
I second the request. 
The tooltips should especially be shown in the Bookmarks-Sidebar.
There, you can't foresee the URL in the statusbar since when you click on the
bookmark the URL already starts loading.
*** Bug 244874 has been marked as a duplicate of this bug. ***
tooltip URL doesn't show. Only shows name of bookmark name. Tooltip is slow or 
doesn't show up at all.
*** Bug 250774 has been marked as a duplicate of this bug. ***
I agree, it should definitely show the url, as it would improve usability, the
same applies to the history sidebar as well I think..

This can be done with a very minor change to browser.xul.

Change:
<menuitem class="menuitem-iconic bookmark-item" uri="rdf:*"
 label="rdf:http://home.netscape.com/NC-rdf#Name" 
 image="rdf:http://home.netscape.com/NC-rdf#Icon"
 status="rdf:http://home.netscape.com/WEB-rdf#status"
 statustext="rdf:http://home.netscape.com/NC-rdf#URL" />
To
<menuitem class="menuitem-iconic bookmark-item" uri="rdf:*"
 label="rdf:http://home.netscape.com/NC-rdf#Name" 
 image="rdf:http://home.netscape.com/NC-rdf#Icon"
 status="rdf:http://home.netscape.com/WEB-rdf#status"
 statustext="rdf:http://home.netscape.com/NC-rdf#URL"
 tooltip="btTooltip" />

On a side note, I didn't see this bug till after I implemented this for myself
so I didn't realize the status bar was supposed to show the URL (making this
unnecessary to me), because the status bar has never shown the URL for me. <shrug />
This is just the kind of consistency cleanup & polish that would be nice to get
on the 1.0 train. Vlad, any chance you can look into this for us?
Assignee: p_ch → vladimir
Flags: blocking-aviary1.0?
Whiteboard: no l10n impact
FYI: Bug 256065, which I raised, is similar.
Patch for this should be easy, if someone wants to tackle it; it can probably go
in 1.0 if there's a patch.
Flags: blocking-aviary1.0? → blocking-aviary1.0-
Keywords: helpwanted
Assignee: vladimir → vladimir+bm
*** Bug 275711 has been marked as a duplicate of this bug. ***
Attached patch (Av1-FF) patch (obsolete) — Splinter Review
Patch per comment #8.

Note that bug 244385 is making the tooltip disappear almost immediately.
Attachment #173038 - Flags: review?(mconnor)
Depends on: 244385
Keywords: helpwanted
Attachment #173038 - Flags: review?(mconnor) → review?(vladimir+bm)
(In reply to comment #13)
> Note that bug 244385 is making the tooltip disappear almost immediately.

True, they are related from a user point of view;
yet there is no bugzilla/code "dependency" between them:
issues are separate, fix are separate.

(Jason: I not sure you need to add your name in the file for a one-liner patch,
but that's up to you.)


[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.5) Gecko/20041217]

While this bug is targeted to FF, MAS has the same issue !

Would you mind if I use it to post the MAS counterpart patch ?
(Or would you post it yourself ? Or do you want me to file a separate bug ?)
No longer depends on: 244385
(In reply to comment #14)
> (Jason: I not sure you need to add your name in the file for a one-liner patch,
> but that's up to you.)
Sorry if I shouldn't have included it. This is one of my first patches, and the
documentation doesn't say when it's appropriate to add it. I wouldn't be
insulted if it went it without my name.
> Would you mind if I use it to post the MAS counterpart patch ?
> (Or would you post it yourself ? Or do you want me to file a separate bug ?)
You can do it.
(In reply to comment #15)
> (In reply to comment #14)
> > (Jason: I not sure you need to add your name in the file for a one-liner patch,
> > but that's up to you.)
> Sorry if I shouldn't have included it. This is one of my first patches, and the
> documentation doesn't say when it's appropriate to add it. I wouldn't be
> insulted if it went it without my name.

First patch: new code contributors are always welcomed :-)

Contributor line: I agree with you ... I feel like only code owners, or
significant feature addition contributors, should (= is worth) add their names,
but that's only my guess.

> > Would you mind if I use it to post the MAS counterpart patch ?
> You can do it.

Thanks, I'll do (later).
Severity: minor → enhancement
Flags: review?(vladimir+bm)
Product: Firefox → Mozilla Application Suite
Hardware: PC → All
Version: unspecified → Trunk
Product: Mozilla Application Suite → Firefox
Comment on attachment 173038 [details] [diff] [review]
(Av1-FF) patch

(damn it, bugzilla bug...)
Attachment #173038 - Flags: review?(vladimir+bm)
Attached patch (Bv1-MAS) <navigatorOverlay.xul> (obsolete) — Splinter Review
MAS counterpart.

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a5) Gecko/20041122] (release)
(W98SE)

Neil:
The toolbar (root) items show Title + Url as expected;
With this patch, I get the Title from the menu items too,
but the Url line is missing, which makes this patch useless.
(how) Can I get the Url to show up too ?
Attachment #173330 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #173330 - Attachment description: (Bv1) <navigatorOverlay.xul> → (Bv1-MAS) <navigatorOverlay.xul>
(In reply to comment #18)
>(how) Can I get the Url to show up too ?
That works using the statustext attribute.
Attachment #173330 - Attachment is obsolete: true
Attachment #173330 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #173038 - Attachment description: patch → (Av1-FF) patch
Bv1, with comment 19 suggestion(s).

{{ (quoting comment 13)
Note that bug 244385 is making the tooltip disappear almost immediately.
}}
Attachment #173499 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 173499 [details] [diff] [review]
(Bv1a-MAS) <navigatorOverlay.xul>

Unfortunately each tooltip= attribute creates a separate listener which could
add up to a lot of wasted memory. Fortunately you can put the tooltip on an
ancestor and it will apply to all of its descendants.
Attachment #173499 - Flags: review?(neil.parkwaycc.co.uk) → review-
(In reply to comment #21)
> (From update of attachment 173499 [details] [diff] [review] [edit])
> Fortunately you can put the tooltip on an
> ancestor and it will apply to all of its descendants.

I'm guessing here :-/
I tried to move |tooltip="btTooltip"|
from |<rule> / class="menuitem-iconic bookmark-item"|
to |<rule iscontainer="true"> / class="menu-iconic bookmark-item"|
which +/- works, but not at all for the first "root popups" from the toolbar...

I need a more detailed/code help on this: helpwanted...
Flags: blocking-aviary1.1?
*** Bug 295318 has been marked as a duplicate of this bug. ***
This is variant 1 of how to resolve this bug.It simply adds
'tooltip="btTooltip"' to the bookmark-items inside a bookmark-toolbar folder.

Advantages: tooltips are shown exclusively on bookmarks.

Disadvantages: memory-consuming as was pointed out in comment #c21.
This is variant 2 of how to resolve this bug. It simply adds
'tooltip="btTooltip"' to the hbox which contains the bookmark-items and folders
and to the hbox which contains the overflowing items inside the
bookmark-toolbar.

Instead, it could also been simply added to the stack, but I was concerned
about the toolbarbutton with class="bookmark-item bookmarks-toolbar-customize".
In my solution, the tooltip attribute does not apply to this certain button.

Advantages: low memory consumption.

Disadvantages: tooltips are also shwon for folders (repeats the name of the
folder), and for the 'Open in Tabs' entry inside a folder.

Remarks: this all aplies only to the bookmark-toolbar, but not to items in the
bookmark menu!
(In reply to comment #25)
>Disadvantages: tooltips are also shwon for folders (repeats the name of the
>folder), and for the 'Open in Tabs' entry inside a folder.
You could always tweak the code to ignore elements without URLs.
(In reply to comment #26)

> You could always tweak the code to ignore elements without URLs.

You mean by tweaking the fillInBTTooltip() (bookmarksMenu.js, line 1019) function?

Something along the lines of a "if (!tipElement.hasAttribute('statustext'))
return;"?
There already is a test there, which is what makes the labels appear...
Ok, I should have a patch ready by the end of tomorrow.
Here is the new version of the tooltip fix for bookmark items inside bookmark
folders in the bookmark toolbar.

I chose the less memory hungry variant, and modified the fillInBTTooltip
(bookmarksMenu.js) such that no tooltips are shown when hovering over some
other non-bookmark item in the toolbar (e.g. a folder itself, or the 'Open in
Tabs' item inside a folder).

Although there are less listeners created, it does not harm performance, since
we don't enter the fillInBTTooltip function every time we hover over an item,
and the tooltip event is only triggered after the specified tooltip timeout.

Please note that this patch does not cover the tooltip issues with the
bookmarks sidebar!
Attachment #184520 - Attachment is obsolete: true
Attachment #184523 - Attachment is obsolete: true
Comment on attachment 173038 [details] [diff] [review]
(Av1-FF) patch

this is bad, for the reasons Neil outlined in his comment.
Attachment #173038 - Flags: review?(vladimir+bm) → review-
Comment on attachment 184751 [details] [diff] [review]
(Av3a-FF) Makes tooltips appear also on bookmarks which are nested inside a folder in the bookmarks toolbar


>-    if (!title && !url) {
>-      // bail out early if there is nothing to show
>+    if (!url || (!title && !url)) {
>+      // bail out early if there is either no url (i.e. if tipElement is not a bookmark, e.g.
>+      // a bookmark folder or the 'Open in Tabs' item) or if neither title nor url are
>+      // present (i.e. tipElement is e.g. an empty menu entry or some other strange thing);
>+      // if only a url but no title or both is present, we want to show the tooltip
>       return false;
>     }

when would we ever hit the second case?  The following is sufficient I think.

// if there isn't a url, don't show a tooltip
if (!url)
  return false;
would be nice to have, but not a blocker.
Flags: blocking-aviary1.1? → blocking-aviary1.1-
*** Bug 301834 has been marked as a duplicate of this bug. ***
Assignee: vladimir+bm → nobody
Attachment #173038 - Attachment is obsolete: true
Attachment #184520 - Attachment description: Variant 1: more memory hungry → (Av2) Variant 1: more memory hungry
Attachment #184523 - Attachment description: Variant 2: less memory hungry, but tooltips also on folders and function menu entries → (Av3) Variant 2: less memory hungry, but tooltips also on folders and function menu entries
Attachment #184751 - Attachment description: Makes tooltips appear also on bookmarks which are nested inside a folder in the bookmarks toolbar → (Av3a) Makes tooltips appear also on bookmarks which are nested inside a folder in the bookmarks toolbar
Attached patch (Av3b-FF) (obsolete) — Splinter Review
Av3a, with comment 32 suggestion(s),
and code simplification.

I don't use FF: Could you test/review/check in this patch ? Thanks.
Attachment #184751 - Attachment is obsolete: true
Attachment #202675 - Flags: review?(mconnor)
Attached patch (Av3c-FF) (obsolete) — Splinter Review
Av3b, with a few (unrelated) FF<-SM synchronization.

I don't use FF: Could you test/review/check in this patch ? Thanks.
Attachment #202675 - Attachment is obsolete: true
Attachment #202675 - Flags: review?(mconnor)
Attachment #202679 - Flags: review?(mconnor)
Attached patch (Bv2-SM) (obsolete) — Splinter Review
Bv1a, with comment 21 suggestion(s),
and with a few (unrelated) FF->SM synchronizations;
in other words, I ported Av3c-FF patch (mostly based on AndreasW's Av3a-FF !).

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20051110 SeaMonkey/1.5a] (nightly) (W98SE)

I tested it successfully:
Tooltips show up for "URI items" on Personal Toolbar and inside its folders;
and not for folders/groups/separators.

As a sidenote,
*Bookmarks _menu_ still does not show any tooltip.
*SideBar show tooltips for "too long" titles only, and does not show the URI.
Could/Should/How theses two be sync'ed with what P.T. will now do !?
Attachment #173499 - Attachment is obsolete: true
Attachment #202690 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #184520 - Attachment description: (Av2) Variant 1: more memory hungry → (Av2-FF) Variant 1: more memory hungry
Attachment #184523 - Attachment description: (Av3) Variant 2: less memory hungry, but tooltips also on folders and function menu entries → (Av3-FF) Variant 2: less memory hungry, but tooltips also on folders and function menu entries
Attachment #184751 - Attachment description: (Av3a) Makes tooltips appear also on bookmarks which are nested inside a folder in the bookmarks toolbar → (Av3a-FF) Makes tooltips appear also on bookmarks which are nested inside a folder in the bookmarks toolbar
Attachment #202690 - Attachment description: (Bv2-MAS) → (Bv2-SM)
Comment on attachment 202690 [details] [diff] [review]
(Bv2-SM)

I've just realized that the old fillInBTTooltip was written that way to work with xpfe bookmark groups, which have never had tooltips before... you'll need to undo those changes, sorry.

>+  // loads a bookmark with the mouse middle button
Should be ... middle mouse button

>+      tooltipTitle.removeAttribute("hidden");
>+      tooltipTitle.setAttribute("hidden", "true");
Nit: should be tooltipTitle.hidden = false / true;
Attachment #202690 - Flags: review?(neil.parkwaycc.co.uk) → review-
(In reply to comment #38)
>>+      tooltipTitle.removeAttribute("hidden");
>>+      tooltipTitle.setAttribute("hidden", "true");
>Nit: should be tooltipTitle.hidden = false / true;
Well, considering the first comment, that hardly matters any more!
(In reply to comment #37)
>*Bookmarks _menu_ still does not show any tooltip.
Right, could you add them to both main menu and personal toolbar menubutton?
(In reply to comment #38)
> (From update of attachment 202690 [details] [diff] [review] [edit])
> I've just realized that the old fillInBTTooltip was written that way to work
> with xpfe bookmark groups, which have never had tooltips before... you'll need
> to undo those changes, sorry.

You mean that SM needs to keep |if (!title && !url) {| !? (while |!url| is enough for FF !?)
What is the SM expected behaviour ? (to get a tooltip on items/folders/groups, == all but separators !?)
(In reply to comment #41)
>You mean that SM needs to keep |if (!title && !url) {| !? (while |!url| is
>enough for FF !?)
I guess so.
>What is the SM expected behaviour ? (to get a tooltip on items/folders/groups,
>== all but separators !?)
groups: tooltip shows the label
items: tooltip shows the url, and the label if it is different but not blank.
folders: no tooltip please
(In reply to comment #40)
> (In reply to comment #37)
> >*Bookmarks _menu_ still does not show any tooltip.
> Right, could you add them to both main menu and personal toolbar menubutton?

P.T. menubutton got the tooltip after restoring fillInBTTooltip() per comment 42.

Main menu: I don't know where (and maybe what) a change should be applied :-/ (helpwanted)
(In reply to comment #43)
>P.T. menubutton got the tooltip after restoring fillInBTTooltip() per comment 42.
I don't see how that happens, there's no change to bookmarks-button that I see.
>Main menu: I don't know where (and maybe what) a change should be applied :-/
<menu id="BookmarksMenu"> needs the tooltip too.
Attached patch (Bv3-SM) (obsolete) — Splinter Review
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20051114 SeaMonkey/1.5a] (nightly) (W98SE)

(In reply to comment #44)
> >P.T. menubutton got the tooltip after restoring fillInBTTooltip() per comment 42.
> I don't see how that happens, there's no change to bookmarks-button that I see.

(I wouldn't know, yet they do.)

> >Main menu: I don't know where (and maybe what) a change should be applied :-/
> <menu id="BookmarksMenu"> needs the tooltip too.

Well, that what I was relunctant to guess.
It works, even a little too well for my taste:
The 4 items in its |menupopup| also display a tooltip :-/ Even for the 3rd/disabled item :-//
I let you decide if something can be done about it, or if that's acceptable...
Attachment #202690 - Attachment is obsolete: true
Attachment #203442 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 203442 [details] [diff] [review]
(Bv3-SM)

OK, so this patch is no good.
Attachment #203442 - Flags: review?(neil.parkwaycc.co.uk) → review-
Comment on attachment 202690 [details] [diff] [review]
(Bv2-SM)

This patch is fine, but I'd like the bookmarksMenu change (as in attachment 203442 [details] [diff] [review]) plus the same for the bookmarks-button in navigator.xul, thanks.
Attached patch (Bv4-SM) (obsolete) — Splinter Review
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20051203 SeaMonkey/1.5a] (nightly) (W98SE)

Bv3-SM, with comment 47 suggestion(s),
and after some emailing between Neil and me.

All seems to work as expected,
except folders, for which I guess the auto-opening feature "stops" the tooltip one.
Unless there is a way around that, I feel like this patch is satisfying.
Assignee: nobody → gautheri
Attachment #203442 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #205441 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #205441 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 205441 [details] [diff] [review]
(Bv4-SM)

Almost there, sort of... I don't see a tooltip on the bookmarks button's bookmarks though.
<toolbarbutton type="menu" id="bookmarks-button" class="bookmark-item">

>+    if (title && (title != url)) {
>       tooltipTitle.setAttribute("value", title);
>+      tooltipTitle.hidden = false;
>+    } else
>+      tooltipTitle.hidden = true;
I don't like the "} else" style (both times). I can think of three options:
1. Just put the {}s back (i.e. "} else {")
2. Reverse the if so you can swap the then and the else (i.e. "else {")
3. Always set the attribute, and then use a boolean expression to set the hidden property.
Attachment #205441 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #205441 - Flags: superreview-
Attachment #205441 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #205441 - Flags: review+
(In reply to comment #45)
> (In reply to comment #44)
> > >P.T. menubutton got the tooltip after restoring fillInBTTooltip() per comment 42.
> > I don't see how that happens, there's no change to bookmarks-button that I see.
> 
> (I wouldn't know, yet they do.)

and

(In reply to comment #49)
> (From update of attachment 205441 [details] [diff] [review] [edit])
> Almost there, sort of... I don't see a tooltip on the bookmarks button's
> bookmarks though.
> <toolbarbutton type="menu" id="bookmarks-button" class="bookmark-item">

Neil is right (;-)), I was misunderstanding what he meant,
because I had turn off the Bookmarks button on the P.T..
Attached patch (Bv5-SM) (obsolete) — Splinter Review
Bv4-SM, with comment 49 suggestion(s),
and after some emailing between Neil and me.

This patch now has 3 parts:
*Adds tooltips for bookmarks (== this bug).
*Removes some unwanted "visual features" of the P.T. Bookmarks button popup.
*Does some additionnal code+space cleanup.
Attachment #205441 - Attachment is obsolete: true
Attachment #207285 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #207285 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 207285 [details] [diff] [review]
(Bv5-SM)

Sorry, still not quite right...

>+                     oncommand="BookmarksMenu.loadBookmark(event, this.database)"
Might as well drop in a ; at the end of the JS statement ;-)

>-          <menuitem accesskey="&addCurPageCmd.accesskey;" key="addBookmarkKb" observes="Browser:AddBookmark"/>
>-          <menuitem accesskey="&addCurPageAsCmd.accesskey;" key="addBookmarkAsKb" observes="Browser:AddBookmarkAs"/>
>+          <menuitem accesskey="&addCurPageCmd.accesskey;" observes="Browser:AddBookmark"/>
>+          <menuitem accesskey="&addCurPageAsCmd.accesskey;" observes="Browser:AddBookmarkAs"/>
>           <menuitem id="PT_bookmarks_groupmark" observes="Browser:AddGroupmarkAs"/>
>-          <menuseparator/>
>-          <menuitem accesskey="&manBookmarksCmd.accesskey;" key="manBookmarkKb" observes="Browser:ManageBookmark"/>
>+          <menuitem accesskey="&manBookmarksCmd.accesskey;" observes="Browser:ManageBookmark"/>
See later for possibility of observes/command conversion. Either way the accesskeys don't need to be duplicated here, because the <command>s already have them, and you'll notice that the other copy of the menuitems don't have access keys.

>-    <command id="cmd_handleBackspace"  oncommand="BrowserHandleBackspace();" />
>-    <command id="cmd_handleShiftBackspace"  oncommand="BrowserHandleShiftBackspace();" />
>+    <command id="cmd_handleBackspace" oncommand="BrowserHandleBackspace();"/>
>+    <command id="cmd_handleShiftBackspace" oncommand="BrowserHandleShiftBackspace();"/>
Don't bother changing non-bookmark-related items.

>       <menupopup id="menu_BookmarksPopup"
>                  onpopupshowing="updateGroupmarkMenuitem('bookmarks_groupmark');">
>         <!-- for some reason these don't work as command="" -->
Aha, well the reason for that is that updateGroupmarkMenuitem needs to update Browser:AddGroupmarkAs and not the individual menuitems directly (which will then not need ids).

>-        <menuitem key="addBookmarkKb"   observes="Browser:AddBookmark"/>
>+        <menuitem key="addBookmarkKb" observes="Browser:AddBookmark"/>
>         <menuitem key="addBookmarkAsKb" observes="Browser:AddBookmarkAs"/>
>         <menuitem id="bookmarks_groupmark" observes="Browser:AddGroupmarkAs"/>
>-        <menuitem key="manBookmarkKb"   observes="Browser:ManageBookmark"/>
>+        <menuitem key="manBookmarkKb" observes="Browser:ManageBookmark"/>
The observes= originally lined up vertically with each other, but then the groupmark menuitem spoiled it...

>+    if (!(tooltipElement.hidden = (!title || (title === url))))
The old code only used != , do you have a reason for the === ? Also see below.

>+    if (!(tooltipElement.hidden = !url))
>+      tooltipElement.setAttribute("value", url);
Unfortunately this is too compact... I think two separate statements for hidden and value works best.
Attachment #207285 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #207285 - Flags: superreview-
Attachment #207285 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #207285 - Flags: review-
Attached file (Bv6-SM) [Wrong file] (obsolete) —
Bv5-SM, with comment 52 suggestion(s).

(In reply to comment #52)
> >+    if (!(tooltipElement.hidden = (!title || (title === url))))
> The old code only used != , do you have a reason for the === ?

I reversed the test(s), and '3=' is supposed to be faster than '2='.

Tested on
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20051230 SeaMonkey/1.5a] (nightly) (W98SE)
Attachment #207285 - Attachment is obsolete: true
Attachment #207385 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #207385 - Flags: review?(neil.parkwaycc.co.uk)
Attached patch (Bv6-SM) (obsolete) — Splinter Review
(See comment 53.)
Attachment #207387 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #207387 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #207385 - Attachment description: (Bv6-SM) → (Bv6-SM) [Wrong file]
Attachment #207385 - Attachment is obsolete: true
Attachment #207385 - Attachment is patch: false
Attachment #207385 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #207385 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 207387 [details] [diff] [review]
(Bv6-SM)

>-  const disabled = gBrowser.browsers.length == 1;
>+  const disabled = (gBrowser.browsers.length === 1);
Sorry, we're not changing style here just because of a minuscule perf win. r+sr=me with this put back (and the other === changed too).

> var BookmarksMenu = {
>   _selection:null,
>   _target:null,
>   _orientation:null,
>-  
>+
> 
Might as well just have one blank line here.
Attachment #207387 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #207387 - Flags: superreview+
Attachment #207387 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #207387 - Flags: review+
Bv6-SM, with comment 55 suggestion(s).

Keeping
{{
(Bv6-SM)   	 patch   	2006-01-02 16:50 PST  	16.46 KB  	neil.parkwaycc.co.uk: review+
neil.parkwaycc.co.uk: superreview+ 
}}
Attachment #207387 - Attachment is obsolete: true
Attachment #207816 - Flags: superreview+
Attachment #207816 - Flags: review+
Comment on attachment 207816 [details] [diff] [review]
(Bv6a-SM)
[Checked in: Comment 57]


Check in: { 2006-01-08 11:45	bugzilla%standard8.demon.co.uk }
Attachment #207816 - Attachment description: (Bv6a-SM) → (Bv6a-SM) [Checked in: Comment 57]
Attachment #207816 - Attachment is obsolete: true
Attachment #207816 - Flags: approval1.8.1?
Comment on attachment 207816 [details] [diff] [review]
(Bv6a-SM)
[Checked in: Comment 57]

Neil, I'd like to submit this patch for (Gv1.8 =) SMv1.1 branch, but I'm not too sure on how to proceed...
Attachment #207816 - Flags: approval1.8.1? → branch-1.8.1?(neil)
Attachment #207816 - Flags: branch-1.8.1?(neil) → branch-1.8.1+
*** Bug 325988 has been marked as a duplicate of this bug. ***
Will there also be a pref to switch bookmarks tooltips off?
I mean without turning off all tooltips.
Bv6a-SM-181, unbitrotted for checkin to current 1.8.1 branch.
Comment on attachment 220752 [details] [diff] [review]
(Bv6a-SM-181)
[Checkin: Comment 62]


Checkin: { 2006-05-05 06:53	bugzilla%standard8.demon.co.uk }
Attachment #220752 - Attachment description: (Bv6a-SM-181) → (Bv6a-SM-181) [Checkin: Comment 62]
Attachment #220752 - Attachment is obsolete: true
Whiteboard: no l10n impact → [no l10n impact] [SM fixed; FF waiting for review]
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8) Gecko/20060505 SeaMonkey/1.1a] (tinderbox-builds, 2006050514) (W98SE)

V.Fixed for SeaMonkey on 1.8.1 branch.
Whiteboard: [no l10n impact] [SM fixed; FF waiting for review] → [no l10n impact] [verified-seamonkey1.1a] [SM fixed; FF waiting for review]
sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs, filter on "beltznerLovesGoats" to get rid of this mass change
QA Contact: mconnor → bookmarks
*** Bug 354407 has been marked as a duplicate of this bug. ***
Hey this Bug seems to have been forgotten about, but I'd suggest it be nominated as a priority ("blocking" on the trunk) due to the introduction of Places in the upcoming Firefox 3.x

Firefox 3.x introduces a 'Places' toolbar button (it's actually a bookmarks folder), and through the use of sub-folders within it, allows users to quickly see which sites they visited recently, which ones they've been to most, which ones they've tagged recently, etc. And while users who have always made use of their bookmarks toolbar are used to seeing a tooltip with Title & URL when an item is hovered over, they'll be wondering why the heck their Places items don't show that information too.

Therefore as long as this Bug remains unresolved, the current situation is one of denied potential and great inconsistency. Consider the (I suspect) fact that the majority of Firefox users likely haven't ever created & used a bookmarks toolbar folder before, but are being given one now, and it means ALL Firefox users who make use of 3's highly-touted and long-awaited innovation will be exposed to this Bug. Therefore I believe it's very important this be fixed ASAP.

By the way, there is an extension called "Boox" https://addons.mozilla.org/en-US/firefox/addon/2615 that solved this problem in Firefox-2 beautifully (among other included handy improvements), and even allowed users to select which info to show in the tooltip. I was going to suggest modelling a fix based on that(with permission/cooperation, check licence), but after bumping the extension maxversion to 3.0.*, see that it unfortunately is not working in Minefield.

I should of course mention that tooltips still don't appear at all within the Bookmarks Menu. A tooltip with the page title is not needed here for obvious reasons, but the URL should be being shown. Yes, the pointed-to URL in the Status area of the statusbar is shown, however with enough bookmarks placed in the root bookmarks folder, the rolled-out bookmarks menu COVERS the default 'Status' statusbar area, which is no good - and/or a user may have so many extensions installed which have placed icons in the statusbar, their Status area has been reduced to barely more than a few fingerwidths, in any case not enough for many full-length URLs.
Comment on attachment 202679 [details] [diff] [review]
(Av3c-FF)

This patch has been waiting review (and bitrotting) for 2 years ! Sad :-(
Attachment #202679 - Attachment is obsolete: true
Attachment #202679 - Flags: review?(mconnor)
I can't help anymore: this will need someone who knows (what) Places (wants)...
Assignee: sgautherie.bz → nobody
Status: ASSIGNED → NEW
Whiteboard: [no l10n impact] [verified-seamonkey1.1a] [SM fixed; FF waiting for review] → [SM fixed; FF see comments 67 & 68] [no l10n impact] [verified-seamonkey1.1a]
Attached patch (Av3d-FF) (obsolete) — Splinter Review
FWIW, it seems I had prepared an Av3c-FF sync'ed to Bv6a-SM, at the time.
Attachment #288236 - Attachment is obsolete: true
Attached patch ff-places-v1 (obsolete) — Splinter Review
Adapted for Firefox which now uses the Places toolbar.
Attachment #308255 - Flags: review?(neil)
Comment on attachment 308255 [details] [diff] [review]
ff-places-v1

Sorry, I'm not a Places peer.
Attachment #308255 - Flags: review?(neil)
Attachment #308255 - Flags: review?(mano)
(In reply to comment #72)
> What's the binding change for?

For some reason, the chevron's menupopup does not show the tooltip implicitely, so I had to make it explicitely inherit the tooltip attribute from the binding parent.
Duplicate of this bug: 429177
Any news about this bug?
It's still present on Firefox 3 pre
Flags: blocking-firefox3?
Last posted patch (ff-places-v1) works perfectly.
Fixed a cosmetic problem with the last patch (ff-places-v1).
It didn't show tooltips longer than 70 chars.
Attachment #317903 - Flags: review?(mconnor)
Attached image long tooltips work fine
(In reply to comment #77)
> Fixed a cosmetic problem with the last patch (ff-places-v1).
> It didn't show tooltips longer than 70 chars.
> 

Huh?  Long tooltips show up just fine.  One of the reasons for doing tooltips is that the title often gets cut off in the menu, and the tooltip is a nice way for people to get the title.  65 characters is barely any longer than the current title cutoff.

----

Also, bug 419313 is very similar (almost a dupe), and it was granted wanted+.
This is Andreas Wuest's patch, with one minor change to extend the behavior to the bookmarks menu, like what SeaMonkey currently does (and not limit this to just the bookmark menus found on the personal toolbar).

Mrtb: Tooltips should automatically resize and truncate to what the available space is, so there is no need to truncate them manually.  Given that the motivation is to allow users to view the full title, I think it's best to avoid manual truncation.

Reviewers and drivers: This should be a fairly low-risk patch, because all the functionality is already present in current the code.  All that this patch is doing is "flipping the switch", so to speak, and turning it on for the bookmark menus.  Allowing users to see the full title through a tooltip will be good for usability.
Attachment #318174 - Flags: review?(mconnor)
Flags: blocking-firefox3? → blocking-firefox3-
I see it with the last nightly and with a clean new profile.
Firefox 3 pre cut off tooltip's title when it is longer than 70 chars.
(In reply to comment #80)
> I see it with the last nightly and with a clean new profile.
> Firefox 3 pre cut off tooltip's title when it is longer than 70 chars.
> 

That's an old bug with tooltips in themed Windows.  It doesn't affect Classic (and probably doesn't affect Lin/Mac, though I'm not 100% sure about that).  Note that tooltips already show up for bookmarks directly on the personal toolbar, and if you add that bookmark to Firefox 2's personal toolbar, you'll get the same cutoff effect.  The cutoff isn't character-based, but width-based, so in this particular case, it's 90 characters. 
This bug is about using the bookmark tooltip, which has always existed for bookmarks sitting on the bookmark toolbar, in bookmark menus.

The problem of a long tooltip being cut off in themed Windows (but not in Classic, which explains why I didn't see the problem at first) has been around for a very long time.  The screenshot that I just attached is from Firefox 1.0.  This is a separate unrelated issue, the fix for which is probably somewhere in widgets/src/windows/ and not here in toolkit/.  I'm also not fond of manual cutoffs, because that doesn't solve the problem of the tooltip being cut off--all that it does is give us more control over where it's cut off, and it'll just end up cutting off the tooltip earlier than it would've otherwise been cut off, which would not be desirable.

So let's file this tooltip cutoff thing as a separate Win32 Widget bug (I suspect that there probably already is a bug for it) and focus on this bug here, which is flipping the switch to turn on tooltips for bookmarks in menus.
Filed bug 431325 for that issue
Attachment #317903 - Flags: review?(mconnor)
Attachment #317903 - Attachment is obsolete: true
However the last patch of this bug (ff-places-v1.2) works perfectly.
Attachment #317903 - Attachment description: ff-places-v1.1 → ff-places-v1.1: put width's limit to 65 chars
Attachment #317903 - Attachment description: ff-places-v1.1: put width's limit to 65 chars → ff-places-v1.1: set width's limit to 65 chars
Attachment #318174 - Flags: ui-review?(beltzner)
Attachment #318174 - Attachment description: ff-places-v1.2: extended to bookmarks menu → ff-places-v1.2: extended to bookmarks menu (behavior now identical to SM2, except folders don't get tooltips)
Attached patch ff-places-v2.0 (obsolete) — Splinter Review
Requesting review.
 LOW-Risk patch.
 Summary of patch:
 -- Fixes all the problems still present on firefox...
    * It has the same behavior of SM2 (see attachment 220752 [details] [diff] [review])
    * It display tooltips on all the links, folders and subfolders of bookmarks toolbar
    * It display tooltips also on bookmarks menu (links, folders, subfolders)
    * It doesn't display a tooltip for an items without title and url
Attachment #319555 - Flags: review?(beltzner)
Component: Bookmarks → Places
QA Contact: bookmarks → places
Attachment #319555 - Flags: review?(beltzner) → review?(neil)
Attachment #319555 - Flags: review?(neil) → review?
Attachment #319555 - Flags: review? → review?(gavin.sharp)
Attachment #319555 - Flags: ui-review?(beltzner)
Depends on: 431325
Whiteboard: [SM fixed; FF see comments 67 & 68] [no l10n impact] [verified-seamonkey1.1a] → [FF has patch, needs review][SM fixed/verified-seamonkey1.1a][no l10n impact]
Attachment #318174 - Attachment is obsolete: true
Attachment #318174 - Flags: ui-review?(beltzner)
Attachment #318174 - Flags: review?(mconnor)
does this fix bug 244371?
No, it doesn't
I created this addon https://addons.mozilla.org/it/firefox/addon/7314 that adds this useful feature to Firefox 3. It fixes also bug 431325.
It includes last patch (ff-places-v2.0)
IMHO this feature should be included on Firefox 3.1
Flags: wanted-firefox3.1?
Nice to have, certainly.
Flags: wanted-firefox3.1? → wanted-firefox3.1+
Priority: -- → P3
Duplicate of this bug: 445002
Assignee: nobody → mrtb06
Comment on attachment 308255 [details] [diff] [review]
ff-places-v1

I guess this patch is obsolete.
Attachment #308255 - Attachment is obsolete: true
Attachment #308255 - Flags: review?(mano)
Blocks: 419313
Status: NEW → ASSIGNED
Blocks: 445000
Blocks: 320495
Summary: Bookmarks should always show tooltip when hovering with mouse → Bookmarks/RSS should always show tooltip when hovering with mouse
Summary: Bookmarks/RSS should always show tooltip when hovering with mouse → Bookmarks/RSS items should always show tooltip when hovering with mouse
Requesting review.
 LOW-Risk patch.
 Summary of patch:
 -- Fixes all the problems still present on firefox...
    * It has the same behavior of SM2 (attachment 220752 [details] [diff] [review])
    * It display tooltips on all the links, folders and subfolders of bookmarks
toolbar
    * It display tooltips also on bookmarks menu (links, folders, subfolders)
    * It doesn't display a tooltip for an items without title and url

Now tooltips are shown also on history menù
Attachment #319555 - Attachment is obsolete: true
Attachment #341312 - Flags: ui-review?(beltzner)
Attachment #341312 - Flags: review?(mconnor)
Attachment #319555 - Flags: ui-review?(beltzner)
Attachment #319555 - Flags: review?(gavin.sharp)
Comment on attachment 341312 [details] [diff] [review]
ff-places-2.1 (rev 0) refresh because of bug 431325 landing

>Index: browser/components/places/content/toolbar.xml
>===================================================================
>RCS file: /cvsroot/mozilla/browser/components/places/content/toolbar.xml,v
>retrieving revision 1.157
>diff -u -r1.157 toolbar.xml
>--- browser/components/places/content/toolbar.xml	28 May 2008 19:04:30 -0000	1.157
>+++ browser/components/places/content/toolbar.xml	1 Oct 2008 17:02:48 -0000
>@@ -78,6 +78,7 @@
>                            collapsed="true"
>                            onpopupshowing="chevronPopupShowing(event);">
>           <xul:menupopup anonid="chevronPopup"
>+                         xbl:inherits="tooltip"
> #ifndef XP_MACOSX
>                          context="placesContext"
> #endif
>Index: browser/base/content/browser-menubar.inc
>===================================================================
>RCS file: /cvsroot/mozilla/browser/base/content/browser-menubar.inc,v
>retrieving revision 1.159
>diff -u -r1.159 browser-menubar.inc
>--- mozilla/browser/base/content/browser-menubar.inc	19 Apr 2008 07:36:39 -0000	1.159
>+++ mozilla/browser/base/content/browser-menubar.inc	1 Oct 2008 16:56:31 -0000
>@@ -351,7 +351,8 @@
>               <menupopup id="goPopup"
>                          type="places"
>                          onpopupshowing="HistoryMenu.onPopupShowing(this);"
>-                         place="place:type=0&amp;sort=4&amp;maxResults=10">
>+                         place="place:type=0&amp;sort=4&amp;maxResults=10"
>+                         tooltip="btTooltip">
>                 <menuitem id="historyMenuBack"
>                           label="&backCmd.label;"
> #ifdef XP_MACOSX
>@@ -407,7 +408,8 @@
>                openInTabs="children"
>                oncommand="BookmarksEventHandler.onCommand(event);"
>                onclick="BookmarksEventHandler.onClick(event);"
>-               onpopupshowing="BookmarksEventHandler.onPopupShowing(event);">
>+               onpopupshowing="BookmarksEventHandler.onPopupShowing(event);"
>+               tooltip="btTooltip">
>       <menuitem label="&bookmarkThisPageCmd.label;"
>                 command="Browser:AddBookmarkAs" key="addBookmarkAsKb"/>
>       <menuitem id="subscribeToPageMenuitem"
>Index: browser/base/content/browser-places.js
>===================================================================
>RCS file: /cvsroot/mozilla/browser/base/content/browser-places.js,v
>retrieving revision 1.127
>diff -u -r1.127 browser-places.js
>--- mozilla/browser/base/content/browser-places.js	7 May 2008 10:14:51 -0000	1.127
>+++ mozilla/browser/base/content/browser-places.js	1 Oct 2008 17:15:37 -0000
>@@ -744,31 +744,30 @@
>   },
> 
>   fillInBTTooltip: function(aTipElement) {
>-    // Fx2XP: Don't show tooltips for bookmarks under sub-folders
>-    if (aTipElement.localName != "toolbarbutton")
>-      return false;
>-
>-    // Fx2XP: Only show tooltips for URL items
>-    if (!PlacesUtils.nodeIsURI(aTipElement.node))
>+    if (!aTipElement.node)
>       return false;
>+      
>+    var title = aTipElement.label;
>+    
>+    // Show URL only for links
>+    if (PlacesUtils.nodeIsURI(aTipElement.node))
>+      var url = aTipElement.node.uri; 	
> 
>-    var url = aTipElement.node.uri;
>-    if (!url) 
>+    // Don't show a tooltip without any data.    
>+    if (!title && !url) 
>       return false;
> 
>-    var tooltipUrl = document.getElementById("btUrlText");
>-    tooltipUrl.value = url;
>-
>-    var title = aTipElement.label;
>     var tooltipTitle = document.getElementById("btTitleText");
>-    if (title && title != url) {
>-      tooltipTitle.hidden = false;
>-      tooltipTitle.value = title;
>-    }
>-    else
>-      tooltipTitle.hidden = true;
>+    tooltipTitle.hidden = !title || (title == url);
>+    if(!tooltipTitle.hidden)
>+      tooltipTitle.textContent = title;
>+      
>+    var tooltipUrl = document.getElementById("btUrlText");
>+    tooltipUrl.hidden = !url;
>+    if (!tooltipUrl.hidden) {
>+      tooltipUrl.value = url;
> 
>-    // show tooltip
>+    // Show tooltip
>     return true;
>   }
> };
>

Little bug on patch, this is the correct one.
Attached patch ff-places-2.1 (rev 1) bugfix (obsolete) — Splinter Review
Sorry, the previous patch was incorrect, this is the correct one.
Attachment #341312 - Attachment is obsolete: true
Attachment #341315 - Flags: ui-review?(beltzner)
Attachment #341315 - Flags: review?(mconnor)
Attachment #341312 - Flags: ui-review?(beltzner)
Attachment #341312 - Flags: review?(mconnor)
Another little fix added.
Attachment #341315 - Attachment is obsolete: true
Attachment #341469 - Flags: ui-review?(beltzner)
Attachment #341469 - Flags: review?(mconnor)
Attachment #341315 - Flags: ui-review?(beltzner)
Attachment #341315 - Flags: review?(mconnor)
Target Milestone: --- → Firefox 3.1
Attachment #341312 - Attachment description: ff-places-2.1 refresh because of bug 431325 landing → ff-places-2.1 (rev 0) refresh because of bug 431325 landing
Attachment #341315 - Attachment description: ff-places-2.1 (rev 1) refresh because of bug 431325 landing → ff-places-2.1 (rev 1) bugfix
a drive-by review

-    // Fx2XP: Don't show tooltips for bookmarks under sub-folders
-    if (aTipElement.localName != "toolbarbutton")
-      return false;
-
-    // Fx2XP: Only show tooltips for URL items
-    if (!PlacesUtils.nodeIsURI(aTipElement.node))
+    if (!aTipElement.node)
       return false;
+      

nit: trailing spaces

+    var title = aTipElement.label;

why using label instead of aTipElement.node.title? for nodes without a title we generate a special cropped title from the uri. so using the label, when you compare title and uri, it will be false in this case.

+    var title = aTipElement.label;
+    

nit: trailing spaces, there are others later, you should fix them in all the patch

+    if (!tooltipUrl.hidden) {
+      tooltipUrl.value = url;
 
where is the closing brace? probably you should remove this "{"
(In reply to comment #98)
> a drive-by review
> 
> -    // Fx2XP: Don't show tooltips for bookmarks under sub-folders
> -    if (aTipElement.localName != "toolbarbutton")
> -      return false;
> -
> -    // Fx2XP: Only show tooltips for URL items
> -    if (!PlacesUtils.nodeIsURI(aTipElement.node))
> +    if (!aTipElement.node)
>        return false;
> +      
> 
> nit: trailing spaces
Ok

> +    var title = aTipElement.label;
> 
> why using label instead of aTipElement.node.title? for nodes without a title we
> generate a special cropped title from the uri. so using the label, when you
> compare title and uri, it will be false in this case.
I have never had this problem with this patch. 
However, ok.

> +    var title = aTipElement.label;
> +    
> 
> nit: trailing spaces, there are others later, you should fix them in all the
> patch
> 
> +    if (!tooltipUrl.hidden) {
> +      tooltipUrl.value = url;
> 
> where is the closing brace? probably you should remove this "{"
Ok.
Thanks for this.
Attachment #341469 - Attachment is obsolete: true
Attachment #341611 - Flags: review?
Attachment #341469 - Flags: ui-review?(beltzner)
Attachment #341469 - Flags: review?(mconnor)
+    // Don't show a tooltip without any data.    
+    if (!title && !url) 

still a couple trailing spaces on these 2 lines. then you can ask review to a peer (mano, dietrich, gavin or mconnor)
Attachment #341611 - Flags: review? → review?(mconnor)
Comment on attachment 341611 [details] [diff] [review]
ff-places-2.2, changes based on mak77 review

We should better ask Gavin to get a quicker review.
Attachment #341611 - Flags: review?(mconnor) → review?(gavin.sharp)
Comment on attachment 341611 [details] [diff] [review]
ff-places-2.2, changes based on mak77 review

>Index: base/content/browser-places.js

>+    var title = aTipElement.node.title;
>+    // Show URL only for links
>+    if (PlacesUtils.nodeIsURI(aTipElement.node))
>+      var url = aTipElement.node.uri;
>+    // Don't show a tooltip without any data.    
>+    if (!title && !url)

This means we'll show tooltips for folders (which have a title), which is undesirable. I don't know why it was decided to start showing tooltips for items without URLs.

(There's still a lot of trailing whitespace in this patch, and it didn't apply cleanly.)
Attachment #341611 - Flags: review?(gavin.sharp) → review-
(In reply to comment #102)
> (From update of attachment 341611 [details] [diff] [review])
> >Index: base/content/browser-places.js
> 
> >+    var title = aTipElement.node.title;
> >+    // Show URL only for links
> >+    if (PlacesUtils.nodeIsURI(aTipElement.node))
> >+      var url = aTipElement.node.uri;
> >+    // Don't show a tooltip without any data.    
> >+    if (!title && !url)
> 
> This means we'll show tooltips for folders (which have a title), which is
> undesirable. I don't know why it was decided to start showing tooltips for
> items without URLs.
> 
> (There's still a lot of trailing whitespace in this patch, and it didn't apply
> cleanly.)

Ok.
Attachment #341611 - Attachment is obsolete: true
Attachment #341830 - Flags: review?(gavin.sharp)
Attachment #341830 - Flags: review?(gavin.sharp) → review+
Comment on attachment 341830 [details] [diff] [review]
ff-places-2.3: no more trailing spaces and tooltips for folders

>Index: base/content/browser-places.js

>+    var tooltipTitle = document.getElementById("btTitleText");
>+    tooltipTitle.hidden = !title || (title == url);
>+    if (!tooltipTitle.hidden)
>+      tooltipTitle.textContent = title;

Why textContent instead of value?
It's used to wrap the title's label if it's too long to be show.
See bug 431325 for more details.
Shouldn't you use it for the tooltip as well, then?
The previous one was based on cvs code (firefox 3.0).
Attachment #341830 - Attachment is obsolete: true
It's the same indentical patch but based on hg code.
Keywords: checkin-needed
(In reply to comment #106)
> Shouldn't you use it for the tooltip as well, then?

I meant "URL" instead of tooltip, but Gabriele pointed out on IRC that it's cropped so it doesn't matter.
Attachment #220752 - Attachment is obsolete: false
Attachment #207816 - Attachment is obsolete: false
Attachment #342437 - Attachment description: ff-places-2.4: based on hg code → [Check-in needed] final patch
Attachment #341830 - Attachment is obsolete: false
Whiteboard: [FF has patch, needs review][SM fixed/verified-seamonkey1.1a][no l10n impact] → [FF has patch, needs checkin][SM fixed/verified-seamonkey1.1a][no l10n impact]
pushed
http://hg.mozilla.org/mozilla-central/rev/b2248e8b2a74
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [FF has patch, needs checkin][SM fixed/verified-seamonkey1.1a][no l10n impact] → [FF has patch][SM fixed/verified-seamonkey1.1a][no l10n impact]
Keywords: checkin-needed
Target Milestone: Firefox 3.1 → Firefox 3.1b2
As what I can see is that it doesn't work for the bookmarks menu on OS X. Don't we have a chance to show the titles there because it's a native menu?
Duplicate of this bug: 419313
Duplicate of this bug: 320495
No longer blocks: 419313
(In reply to comment #111)
> As what I can see is that it doesn't work for the bookmarks menu on OS X. Don't
> we have a chance to show the titles there because it's a native menu?

Dietrich, can you give me an answer to that question? If not, whom I could ask for?
(In reply to comment #111)
> As what I can see is that it doesn't work for the bookmarks menu on OS X. Don't
> we have a chance to show the titles there because it's a native menu?

After some investigation it doesn't seem to be possible with OS X. At least if there will be a way we should handle this in its own bug.

Verfied the patch on trunk with:

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b2pre) Gecko/20081017 Minefield/3.1b2pre

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081017 Minefield/3.1b2pre ID:20081017020232
Status: RESOLVED → VERIFIED
Flags: in-litmus?
actually when draggging these new tooltips are causing a lot of headache since as soon as one start dragging a place node in a popup the tooltip appear and as soon as the mouse touches the tooltip all popups are dismissed.

if bug 312852 is not fixed before the release i would ask to backout this
Duplicate of this bug: 421638
Depends on: 465363
Blocks: 451022
Why this bug hasn't been fixed for 1.9.0 yet? We still have it on 1.8.1.
(In reply to comment #118)
> Why this bug hasn't been fixed for 1.9.0 yet? We still have it on 1.8.1.

Because it's not a 1) stability fix, 2) security fix, or 3) fix for regression from previous *maintenance* release. And it's never been nominated for anything.
Test case https://litmus.mozilla.org/show_test.cgi?id=6752 was updated for regression testing.
Flags: in-litmus? → in-litmus+
Disregard the previous test case link, this is the test case that was updated for regression testing.

https://litmus.mozilla.org/show_test.cgi?id=6376
Depends on: tooltips
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
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
Blocks: tooltips
No longer depends on: tooltips
You need to log in before you can comment on or make changes to this bug.