Closed Bug 27828 Opened 25 years ago Closed 24 years ago

TITLE attribute should show tooltip

Categories

(SeaMonkey :: UI Design, enhancement, P3)

Product:
SeaMonkey
Component:
UI Design
Type:
enhancement

Tracking

(Not tracked)

Status:
VERIFIED WORKSFORME

People

(Reporter: cu, Assigned: bugs)

References

Details

(Keywords: crash, Whiteboard: [nsbeta2+])

Attachments

(2 files)

Patch to nsXULPopupListener to fix dangling node ref. crash
4.78 KB, patch
Details | Diff | Splinter Review
Hack for super-verbose tooltips on shift-mouseover; DO NOT CHECK IN!
7.23 KB, patch
Details | Diff | Splinter Review 
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".
Tom -- this is one of yours, right?
Assignee: rickg → joki
*** Bug 35373 has been marked as a duplicate of this bug. ***
is this really an enhancement? If it's HTML4 spec, I would call it a bug if 
Mozilla didn't comply.
This looks like a duplicate of the ever-expansive bug 1995, but perhaps it
should be kept separate.
Summary: HREF TITLE does not work → TITLE attribute should show tooltip
*** Bug 37981 has been marked as a duplicate of this bug. ***
OS: Windows 2000 → All
Hardware: PC → All
The spec says that TITLE "may ... be rendered as a tool tip." Not a requirement. 
M20.
Target Milestone: --- → M20
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
Assignee: joki → roc+moz
Blocks: html4.01
Adding keywords '4xp' (since competitors support this) and 'html4'.
Keywords: 4xp, html4
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.
Keywords: html4
html4 keyword is good for anything in HTML4, even if it was in earlier specs 
too.
Keywords: html4
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"
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?
Component: HTML Element → XP Apps: GUI Features
Keywords: nsbeta2, patch
Whiteboard: have fix, awaiting review and checkin
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?

assigning to ben so this gets checked in.
Assignee: roc+moz → ben
Status: ASSIGNED → NEW
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.
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...
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.
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 :-).
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.

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.
Keywords: crash
Whiteboard: have fix, awaiting review and checkin → [nsbeta2+]have fix, awaiting review and checkin
Putting on [nsbeta2+] radar.  We chose leaks for beta2, please fix the crash.
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.

patch checked in (sorry for the delays). does anything else have to happen with 
this bug?

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
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
fix checked in. 
Confirm working Linux BuildID 2000071120
Fixed in the July 11 build.
Status: RESOLVED → VERIFIED
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.
Tim, please file a separate bug for that, it Shouldn't Happen.
Blocks: 45380
Blocks: 19425
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.
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..
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.
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...
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.
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.
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.
actually it was set as WONTFIX
bad me who reopened, but i'd really like a second opinion on that one.
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.
Blocks: 45375
Keywords: 4xp, html4, patch
Whiteboard: [nsbeta2+]have fix, awaiting review and checkin → [nsbeta2+]
People in newsgroups are complaining about the Location: tooltip. Should we 
reopen this bug, file another, or just wait for ben to reappear?
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."

and bug 45530 "Tooltips need to time out"
Ben, please, can you stop this flow of bugs by reverting your Location patch?
* 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.

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.
In M16 (NT) no Tool tip is displayed.

Try the test URL from bug 45375. (I would add it but don't have privs.)
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
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.
Status: REOPENED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → WORKSFORME
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.
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.
Status: RESOLVED → VERIFIED
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.
Tooltip doesn't appear with 2000120306-Mtrunk/Linux.
This bug should be reopened.
No longer blocks: 45375
Product: Core → Mozilla Application Suite
Component: XP Apps: GUI Features → UI Design
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: