Last Comment Bug 27828 - TITLE attribute should show tooltip
: TITLE attribute should show tooltip
Status: VERIFIED WORKSFORME
[nsbeta2+]
: crash
Product: SeaMonkey
Classification: Client Software
Component: UI Design (show other bugs)
: Trunk
: All All
: P3 enhancement (vote)
: ---
Assigned To: Ben Goodger (use ben at mozilla dot org for email)
: Chris Petersen
Mentors:
: 35373 37981 (view as bug list)
Depends on:
Blocks: metadata html4.01 19425 45380
  Show dependency treegraph
 
Reported: 2000-02-15 10:07 PST by Cuneyt
Modified: 2008-07-31 04:03 PDT (History)
15 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch to nsXULPopupListener to fix dangling node ref. crash (4.78 KB, patch)
2000-06-16 10:05 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
no flags Details | Diff | Splinter Review
Hack for super-verbose tooltips on shift-mouseover; DO NOT CHECK IN! (7.23 KB, patch)
2000-07-14 13:33 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
no flags Details | Diff | Splinter Review

Description Cuneyt 2000-02-15 10:07:56 PST
A sample link like

<a href="foo/" title="bar">foo</a>

should display a "bar" (like a tooltip) when mouse points to this link.

It's a part of the specs (http://www.w3.org/TR/html4/struct/links.html#h-12.1.4) 
and the same thing works in IE4,5 and Opera 3.6. This feature is much more 
flexible with lots of JavaScript onMouseOver's, because I don't like that I 
can't see the target URL when I point to a link and status bar says a stupid 
phrase like "go to blabla".
Comment 1 rickg 2000-03-06 00:41:59 PST
Tom -- this is one of yours, right?
Comment 2 Jerry Baker 2000-04-10 14:30:20 PDT
*** Bug 35373 has been marked as a duplicate of this bug. ***
Comment 3 Joseph Elwell 2000-04-11 11:52:10 PDT
is this really an enhancement? If it's HTML4 spec, I would call it a bug if 
Mozilla didn't comply.
Comment 4 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2000-04-20 19:37:10 PDT
This looks like a duplicate of the ever-expansive bug 1995, but perhaps it
should be kept separate.
Comment 5 Braden 2000-05-03 01:24:16 PDT
*** Bug 37981 has been marked as a duplicate of this bug. ***
Comment 6 ekrock's old account (dead) 2000-05-20 23:18:40 PDT
The spec says that TITLE "may ... be rendered as a tool tip." Not a requirement. 
M20.
Comment 7 Hixie (not reading bugmail) 2000-05-21 02:20:35 PDT
Robert -- since you are working on this for bug 1995, I'm going to reassign this
to you. That way once your patch is checked in you'll have the pleasure of being
able to mark a bug FIXED. :-)

Anyone who's interested can find Robert's patch here:
   http://bugzilla.mozilla.org/showattachment.cgi?attach_id=8779
Comment 8 Antti Näyhä 2000-06-06 02:37:09 PDT
Adding keywords '4xp' (since competitors support this) and 'html4'.
Comment 9 Antti Näyhä 2000-06-06 02:39:24 PDT
Oops, this has been around since HTML 2.0 and definitely isn't an HTML4-only 
feature.  Wonder what was I thinking? :-)  Removing 'html4' keyword.
Comment 10 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2000-06-06 09:48:06 PDT
html4 keyword is good for anything in HTML4, even if it was in earlier specs 
too.
Comment 11 Antti Näyhä 2000-06-07 00:59:32 PDT
Hmm?  http://bugzilla.mozilla.org/describekeywords.cgi has this to say about 
the 'html4' keyword:

"Bug in support for HTML 4.0 only elements (whether required for compliance or 
not), i.e. HTML elements or attributes that were first introduced as part of the 
HTML 4.0 Specification, and not included in HTML 3.2"
Comment 12 Hixie (not reading bugmail) 2000-06-15 12:35:02 PDT
TITLE tooltips patch version 5 is now available:
   http://bugzilla.mozilla.org/showattachment.cgi?attach_id=10191

Thanks Robert.

PDT: We have a fix ready to be checked in, it just needs somebody to review it
and someone to check it in (contributor does not have CVS access). This was 
marked nsbeta2- when it was in bug 1995.

ben@netscape.com: are you the right person to review this patch? Who is?
Comment 13 Hixie (not reading bugmail) 2000-06-15 12:36:45 PDT
Mike: You were looking at checking in the patch when it was in bug 1995, are
you still able to review and checkin this patch?
Comment 14 Mike Pinkerton (not reading bugmail) 2000-06-15 12:42:20 PDT
assigning to ben so this gets checked in.
Comment 15 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2000-06-15 14:56:17 PDT
There is a bug in the tooltip code: if the tooltip target DOM element is deleted 
during the interval between the mouse hovering over the element and the tooltip 
popping up, then we crash.

The TITLE tooltip patch is triggering this bug annoyingly often (e.g. you click 
Back, accidentally hover mouse over the current page, new page loads, boom). I'd 
fix the underlying bug, but I'm not sure how to detect that the DOM element has 
been deleted. I could put an owning reference in the tooltip handler, but I 
still don't know how to detect that the other real references have gone away.

Until this issue is fixed, better not check in, especially for nsbeta2.
Comment 16 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2000-06-15 20:40:42 PDT
The crash seems to be (in nsXULPopupListener.cpp) because mPossibleTooltipNode 
is a weak reference - if it were a strong reference you could probably check 
if the parent still exists to see if it was still in the document.  (One reason 
I could see to make it a weak reference is in case mPossibleTooltipNode is an 
ancestor of mElement (or something else that owns the nsXULPopupListener).  
Perhaps this case should be tested for explicitly, if that was the reason?  
cc:ing pinkerton since he probably knows why he made mPossibleTooltipNode a weak 
reference.)

The crash I saw was:
#0  0x80dc527 in ?? ()
#1  0x40455db8 in nsCOMPtr<nsIDOMElement>::assign_from_helper (this=0xbffff760, 
helper=@0xbffff754, aIID=@0x404b3bc0) at ../../../dist/include/nsCOMPtr.h:856
#2  0x40455fcd in nsCOMPtr<nsIDOMElement>::nsCOMPtr (this=0xbffff760, 
helper=@0xbffff754) at ../../../dist/include/nsCOMPtr.h:564
#3  0x411ef6d4 in XULPopupListenerImpl::sTooltipCallback (aTimer=0x8803628, 
aClosure=0x85309b8) at nsXULPopupListener.cpp:583
#4  0x419c3955 in nsTimerGtk::FireTimeout (this=0x8803628) at nsTimerGtk.cpp:58
etc...
Comment 17 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2000-06-16 10:05:20 PDT
Created attachment 10258 [details] [diff] [review]
Patch to nsXULPopupListener to fix dangling node ref. crash
Comment 18 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2000-06-16 10:07:38 PDT
I made it into a strong reference, and check to see if it belongs to any 
document before displaying the tooltip. I think the reference cannot leak, 
because it will always be dropped when the timer fires or otherwise goes away. 
Anyway, it fixes the crashes.
Comment 19 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2000-06-16 10:08:56 PDT
BTW, I'm sure I could make up a XUL-only testcase, the crashes are not just an 
HTML-tooltip issue. Weak references are dangerous, and here they're being 
shamelessly abused :-).
Comment 20 Mike Pinkerton (not reading bugmail) 2000-06-16 10:16:57 PDT
weak references are perfectly alright....it's the overuse of strong references 
that give us memory leak nightmares. i'm wary of the strong ref here, not wanting 
to introduce cycles. I'll look at the patch this weekend.
Comment 21 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2000-06-16 10:40:56 PDT
Crashes or leaks, pick your poision :-).

Actually, this particular strong ref is fine even if there are cycles (which is 
actually common: frequently the popup listener is attached to the element which 
is the target of the mouse hover event). Once the reference is set, the timer is 
launched and one of four things must happen:
-- We fail to start the timer. The reference is cleared.
-- We cancel the timer. The reference is cleared.
-- The timer fires. The reference is cleared.
-- The nsXULPoupListener is destroyed (along with the reference).
So even if the nsXULPopupListener lives forever, the reference will be cleared 
in 500ms or less.
Comment 22 leger 2000-06-16 13:26:38 PDT
Putting on [nsbeta2+] radar.  We chose leaks for beta2, please fix the crash.
Comment 23 Mike Pinkerton (not reading bugmail) 2000-07-11 10:11:04 PDT
I (finally) reviewed the patch and it looks good to me. I'll apply it and check 
it in today and hopefully we can get this bug taken care of.
Comment 24 Mike Pinkerton (not reading bugmail) 2000-07-11 13:56:21 PDT
patch checked in (sorry for the delays). does anything else have to happen with 
this bug?
Comment 25 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2000-07-11 14:16:01 PDT
Thanks for checking that in.

Now it should be safe to check in the real patch, which enables tooltips for
browser content:
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=10191
Comment 26 Ben Goodger (use ben at mozilla dot org for email) 2000-07-11 17:20:23 PDT
fix checked in. 
Comment 27 Jeff Bailey 2000-07-11 23:30:57 PDT
Confirm working Linux BuildID 2000071120
Comment 28 Chris Petersen 2000-07-12 13:51:00 PDT
Fixed in the July 11 build.

Comment 29 Tim Hill 2000-07-13 01:47:39 PDT
I like the new title tooltips, but I don't like how it shows up as "Location: 
url" on links without a value for "title."  This information is already in the 
statusbar.
Comment 30 Matthew Paul Thomas 2000-07-13 07:17:19 PDT
Tim, please file a separate bug for that, it Shouldn't Happen.
Comment 31 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2000-07-13 08:28:12 PDT
After he checked in my patch, Ben checked in some more changes of his own that 
made the behaviour deviate from Ian's spec. Namely, he added the "Location" and 
"Link will open in" data for all links. He also removed the blank line that was 
supposed to appear between texts. He also introduced a bug on the following 
line:
      while ( !titleText && !summaryText && !XLinkTitleText && !linkTarget && 
tipElement ) {
It also needs to test linkRealHREF.

Personally I don't like these changes, but that's not important. The important 
thing is that we reconcile Ian's spec with the implementation by either changing 
the spec or the implementation. Otherwise we'd better front up and say out loud 
that we're abandoning Ian's spec.
Comment 32 R.K.Aa. 2000-07-14 02:56:02 PDT
Those "uninvited" tooltips (for links without TITLE) are rapidly getting very
annoying.. they appear way outside browser area when long, and cover a TV app i
have running. The result is screen flicker as a refresh is forced - the tooltip
vanish before i can even read it.
But i don't want it there in the first place! Redundant information. At best
those tooltips are "only" in the way for reading normal webcontent, at worst my
whole screen-image jumps. I have to be careful about where i move cursor these
days, and avoid sliding it over links. Was there a bug filed on this yet? I'm
getting sea-sick here..
Comment 33 R.K.Aa. 2000-07-14 08:12:07 PDT
at http://www.macromedia.com/software/flash/ the table summary tags always
display as tooltips now. Is this correct behaviour? (They read "main body of
page" or "page header and crumb trail links" and the likes. It doesn't appear as
the author(s) intended it to be seen.
Comment 34 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2000-07-14 11:37:11 PDT
Ian's spec is quite clear that TABLE SUMMARY should be exposed to the user, and 
I think the HTML4 spec agrees. I don't think this is a problem. Some sites may 
be using the tag inappropriately but there don't seem to be many of them and 
they'll learn.

The Location and Link Will Open In tooltips are the real problem. I'm also 
rapidly coming to hate them.

How about, if the user holds down a modifier key (say "shift"), then we show a 
tooltip giving very detailed information about the DOM element they're mousing 
over? I have a "DOMTips" patch lying around that nicely displays the tags and 
attributes of every DOM node on the path from the moused-over node to the root 
of the document tree. This provides the Location and Link Will Open In 
information plus a lot more for those who want it...
Comment 35 R.K.Aa. 2000-07-14 12:09:53 PDT
The tooltips never go away. They need a timeout. On pages with table summary
tags, there is now no way to avoid that tooltip displaying on page at all times.
It vanish for a brief sec while cursor moves, then returns - and remains. Till
next time cursor is moved. When i've seen its content once - i've seen it. No
need to display the same information 100 more times while i move cursor around
to make tooltip go away so i can read what it covered? If that is the spec, it
might need re-thinking.

In NC a tooltip displays for a certain amount of time, and then automatically
"turns off". You have to deliberatly move over that object once again to trigger
the tooltip to display again. I see that as the desireable behaviour also in
Mozilla.
Comment 36 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2000-07-14 12:16:21 PDT
I agree with you, but that is a different bug. This bug, and Ian's spec, are 
about what kind of tooltips should be displayed and what they should contain. 
You're talking about how tooltips should respond to user input. Please file a 
separate bug on that, if there isn't one already.
Comment 37 R.K.Aa. 2000-07-14 12:37:21 PDT
bug 45497 was already filed about this - and set invalid. I took the liberty of
reopening it. I'll vote for it if needed. This is getting annoying.
Comment 38 R.K.Aa. 2000-07-14 12:39:19 PDT
actually it was set as WONTFIX
bad me who reopened, but i'd really like a second opinion on that one.
Comment 39 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2000-07-14 13:29:28 PDT
See my comments on bug #45497.

As for this bug: I have implemented my suggestion above. I will submit the patch 
for people to try out. It works fairly well. One problem is that it seems 
difficult to determine in XUL whether the shift key is down, unless you have a 
mouse event around, which you don't have when a tooltip is popping up. So I 
hacked it by listening to all mouse events and remembering their shift state.
Comment 40 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2000-07-14 13:33:57 PDT
Created attachment 11425 [details] [diff] [review]
Hack for super-verbose tooltips on shift-mouseover; DO NOT CHECK IN!
Comment 41 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2000-07-15 17:10:34 PDT
People in newsgroups are complaining about the Location: tooltip. Should we 
reopen this bug, file another, or just wait for ben to reappear?
Comment 42 R.K.Aa. 2000-07-16 00:49:16 PDT
Keeping track.. there are now two new bugs on tooltip behaviour:
bug 45497 "Tool Tips remain after loading new page"
bug 45522 "Improve tooltip behavior or add pref to turn them off."
Comment 43 R.K.Aa. 2000-07-16 00:51:02 PDT
and bug 45530 "Tooltips need to time out"
Comment 44 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2000-07-16 07:18:43 PDT
Ben, please, can you stop this flow of bugs by reverting your Location patch?
Comment 45 Matthew Paul Thomas 2000-07-17 20:56:17 PDT
* Bug 45522 is tracking tooltip behavior -- in exactly what situations tooltips
  should disappear. (Those bugs were going to happen anyway, Ben's faux pas just
  exposed them.)
* Filed bug 45733 to get rid of the `Location:' and `Link will open in:'
  tooltips. 
* Filed bug 45735 that we shouldn't show a tooltip for TABLE SUMMARY (Ian's spec
  is wrong, TABLE SUMMARY is only for non-visual browsers).
* If a separate bug needs to be filed to fix the leaks, someone please file it
  and note the bug number here.
* Robert, please file the linkRealHREF bug, and note the bug number here. (I
  can't file it because I don't understand it.:-)
* Robert, if you want your verbose tooltips to be checked in, please file it as a
  separate RFE and note the bug number here.

Apart from those issues, I think this bug is now over. Please adjust your
votes/CCs accordingly. Thankyew.
Comment 46 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2000-07-18 08:18:52 PDT
The linkRealHREF problem is no longer an issue because linkRealHREF has gone 
away.

There are no leaks that I know of.

I'm undecided as to whether the verbose tooltips are useful or not. I won't 
propose an RFE unless someone requests it.
Comment 47 Ariel Shkedi 2000-08-02 17:14:23 PDT
In M16 (NT) no Tool tip is displayed.

Try the test URL from bug 45375. (I would add it but don't have privs.)
Comment 48 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2000-08-02 17:23:35 PDT
M16 is old news. TITLE tooltips are working nicely in my build from yesterday's 
tip.

Pull down a nightly build and if it still doesn't work, reopen this again. And 
make sure you try it with a clean install and profile etc.
Comment 49 Ariel Shkedi 2000-08-02 20:02:23 PDT
Downloaded it, works great. I can't seem to mark it as Verified though.

On a side note, the verbose tooltips, I haven't seen what they look like, but I
think it's a great idea - it would help with creating pages, and debugging them.
So consider this a request.
Comment 50 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2000-08-02 20:36:09 PDT
I'll verify for you.

The verbose tooltips aren't likely to be checked in, but you can easily apply 
the patch to your own navigator.js and navigator.xul.
Comment 51 James Green 2000-08-11 05:10:34 PDT
I agree that verbose tooltips should be available, but we need to think about
how to activate them. The shift-key idea is fine, but do we want the
shift-modifier for something else? There's a bug filed elsewhere to ask for
multiple links to be opened at once (in new windows). I guessed that could
happen by shift-clicking a number of links on a page then right-clicking and
selectiong a "Open all in new windows" menu item. Food for thought. This
verbosity issue should be another bug anyway, imho.
Comment 52 Koike Kazuhiko 2000-12-04 04:38:57 PST
Tooltip doesn't appear with 2000120306-Mtrunk/Linux.
This bug should be reopened.

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