Closed Bug 248488 Opened 20 years ago Closed 19 years ago

Improve Tooltip display of long TITLE attribute values

Categories

(Camino Graveyard :: General, defect)

Product:
Camino Graveyard
Component:
General
Platform:
PowerPC
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Camino1.5

People

(Reporter: precision-image, Assigned: jaas)

References

(
URL
)

Details

(Keywords: fixed1.7.7, testcase)

Attachments

(2 files, 4 obsolete files)

test case
434 bytes, text/html
Details
Richer testcase
1.18 KB, text/html
Details 
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7) Gecko/20040623 Camino/0.8
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7) Gecko/20040623 Camino/0.8

This bug seems very similar to PC-bug 247815... however I did not find any
Camino bugs when I searched for the string "vBulletin" so I thought it worth
posting. 

Reproducible: Always
Steps to Reproduce:
1. Go to a vBulletin 3.0.x site that has the thread preview feature turned on by
defult.
2. Run your curosor over a thread title and wait a second or two
3. Camino will show the preview but in such a way as to use only a single row of
text. If the preview is set to display enough characters, the beginning and end
of the preview will get truncated at the screen edges.



Expected Results:  
As a comparison, Safari will show the thread previews in a "box" rather than a
single row, and will use ellipses to show that there is more to be viewed when
the character limit has been reached. Camino should also handle vBulletin Thread
Previews in this way IMO.
Confirmed, but I'm not sure if it's a dup or not. Is our tooltip system distinct
enough that we would have a separate fix?
Status: UNCONFIRMED → NEW
Ever confirmed: true
yeah, this bugs me as well. we should fix it.
Status: NEW → ASSIGNED
Target Milestone: --- → Camino0.9
our toolips are custom. gecko tells us what to show, but now how to do it. we
also don't use the os tooltips for other reasons (bugs which i no longer recall).

so something isn't wrapping correctly. shouldn't be tough to fix.
Assignee: pinkerton → joshmoz
Status: ASSIGNED → NEW
Attached file test case — 
Use this little test page to test the patch.
Attached patch patch that almost works (obsolete) — — Splinter Review 
This patch almost works, but with one annoying bug I can't seem to fix. Apply,
compile, then try the test page I just posted.

1. Move mouse over short string until tooltip pops up
2. Move mouse over long string until tooltip pops up
3. Move mouse off of long string onto blank space
4. Move mouse back over long string until tooltip pops up

The first time you move the mouse over the long string it is incorrectly sized
(right after moving mouse over the short string). The second time it is
correctly sized. Arg.

If someone else could fix this, do a thorough review, and land it before I get
back to it that would be great :) Otherwise I'm just posting this patch here so
I can keep track of it myself.
Take a look pink? Probably something obvious I'm missing because I've been
staring at this for way too long.
Attached patch patch that really does work (obsolete) — — Splinter Review 
This updated patch works in all cases I could think of. Here is what it does:

- smarter sizing. sizes tooltips correctly, wraps them at a certain width.
allows multiple lines of text
- smarter about what it does when a tooltip would display at least partially
offscreen
Attachment #153258 - Attachment is obsolete: true
Attachment #153364 - Flags: superreview?(pinkerton)
Attachment #153364 - Flags: review?(sfraser)
Latest patch looks good and works but might be "too" smart.

Example case of http://www.zeldman.com, see the side "Departments":
1) colophon, contrast, externals, and rss feed all render fine and as they should (Yay, the patch 
worked).
2) contrast-o-meter, essentials, goodies, and pleasure cut their line short. As an example, goodies title 
says "Toys for" on one line and puts "you." on another. The source of the page doesn't show any sign of 
this being a purposeful thing.

Besides the minor cutting of some tooltips/titles short, this patch works.
Nice find. That does need to be fixed.
Attachment #153364 - Flags: superreview?(pinkerton)
Attachment #153364 - Flags: review?(sfraser)
Comment on attachment 153364 [details] [diff] [review]
patch that really does work

>Index: src/browser/ToolTip.h
>===================================================================
>RCS file: /cvsroot/mozilla/camino/src/browser/ToolTip.h,v
>retrieving revision 1.1
>diff -u -p -r1.1 ToolTip.h
>--- src/browser/ToolTip.h	8 Aug 2002 19:36:10 -0000	1.1
>+++ src/browser/ToolTip.h	16 Jul 2004 03:23:44 -0000
>@@ -20,6 +20,7 @@
>  * the Initial Developer. All Rights Reserved.
>  *
>  * Contributor(s): Richard Schreyer
>+ *                 Josh Aas <josha@mac.com>
I thought you where using gmail for Camino related stuff ?

If you need a review set me for reviewer, I'll do it on Monday. I can't before
that date.
I do use gmail for moz related stuff, but for licensing/copyright things I use
my more permanent address.
Attachment #153364 - Flags: review+
Josh, did the patch for this bug work and never get checked in, or something else?

Also, this appears to be a duplicate of bug 167435--which has a much better
summary of the underlying problem than this one, but no patch and a completely
different target milestone....

I made a comment in 167435 not too long ago and found this today while looking
for something else in your blog; if these two are duplicates and this one stays
because of the patch, I recommend at least adding "TITLE" and "tooltip" to this
bug's summary.
*** Bug 167435 has been marked as a duplicate of this bug. ***
Thank you Smokey you are right, did the paperwork to resolve the other one and
chnages this bug title to be like Bug 167435.
Summary: vBulletin (3.0.x) Thread Previews do not display properly → Improve Tooltip display of long TITLE attribute values
The patch was not checked in because of comment #8. It isn't ready.
Hi,

I came to therealization that we just need to not use sizeToFit -- other people
have complianed about it on cocoa-dev.

This patch tries to reimplement it, and is pretty close to fully working, in my
testing. The main problem (check on vBulletin) is that on some multiline
tooltips, the text will actually be cut off at the top by either part of a line
or several lines -- it's as if the text view is moved up so it starts above the
tooltip frame. The real wierd thing here is that usually by moving the mouse to
make the tip reappear, the text will be located correctly.

I hope this helps and I'd like people to try it out for a spin. If I have time
I'll try to squash the remaining issue :)
Still has bugs, not asking for review. Just updating it so it applies if anyone
wants to mess with it. I'm working on it at the moment but I suspect I won't
get far.
Attachment #153364 - Attachment is obsolete: true
i was playing with "patch that really does work" and it with the exception of it
wrapping a few tooltips that should be a single-line, it works SO MUCH BETTER
than what we have now we should really just go ahead and land it and move on
with our lives.
i landed the "updated version of patched that works" on the trunk and i think we
should ship it in 084. Like I said, it's really a LOT better than what we have
now. If we want to work to improve sizeToFit (which after playing with it I
agree has serious issues)) on the trunk, we can, though the i18n issues might be
hairy. 

I'll deal with landing the patch on the 17branch.
Target Milestone: Camino0.9 → Camino1.1
I doubt we're going to fix the problem any time in the near future as sizeToFit
is busted or has unhelpful characteristics, and re-implementing sizeToFit such
that it works with all character sets, text directions and whatnot pretty much
isn't going to happen. The only way this will get fixed any time soon is with an
OS update.
landed on branch. should we just future this?
Keywords: fixed1.7.7
Comment on attachment 178542 [details] [diff] [review]
updated version of "patch that really does work"

landed on trunk and branch, obsoleting and plussing.
Attachment #178542 - Attachment is obsolete: true
Attachment #178542 - Flags: review+
Looks good so far, aside from some of the margins being a few px too large.
(In reply to comment #18)
> i was playing with "patch that really does work" and it with the exception of it
> wrapping a few tooltips that should be a single-line, 

Wow, I see how messed up that underlying function is!  :-(

Have a look at tooltips on Wikipedia articles 
http://en.wikipedia.org/wiki/Camino
is good, as is 
http://en.wikipedia.org/wiki/Template:Europe 

And notice the number of short tootips wrapping strangely: Mozil la,  2 0 0 5, etc.

Long ones do seem to be fixed nicely, except for some wierdness in the second
testcase from bug 167435, attachment id=123085, going upwards offscreen.
yeah |sizeToFit| must have internal rounding errors. It would wrap "Latest
Firefox Nightly Build" but keep "Latest Firefox NigltlY Build" as a single line
-- the only difference being the capital "Y" in Nightly, an even wider string to
boot!

regardless, there's no point pointing out all the places where this falls down,
we know it does, we know there's not a whole heck of a lot we can do about it. 
I'm happy to see multiline tooltips, but it is also worth pointing out (for
future reference, if anything) that HTML 4 actually specifies a different behavior.

Specifically, since TITLE attribute values are like any other attribute value
merely CDATA, <http://www.w3.org/TR/html4/types.html#type-cdata> says UAs are
supposed to replace "...each carriage return or tab with a single space."

Of course, if I'm wrong, I'd be glad to hear about it.
Keywords: testcase
I think tooltips are worse now than they were before. I just got one like:

F
U
D


for every one that's worse, there are 50 that no longer take up the full screen
width. Have fun with sizeToFit. I think we're in a better place than we were.
I checked in some code that measure the string width by hand, adds some magic
numbers, uses that as a min width, and then calls -sizeToFit. It fixes the
examples in the testcase.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
won't setting that minWidth then cause long tooltips to be the whole screen
again?  What am i missing?
ah, i see now. the max width takes care of that. looks better, fixes a lot of
the places where i noticed problems (mozillazine.org, for example). 
(In reply to comment #30)
> I checked in some code that measure the string width by hand, adds some magic
> numbers, uses that as a min width, and then calls -sizeToFit. It fixes the
> examples in the testcase.

Is the fix here of any use towards Firefox bug 45375?
No, this is all Cocoa code.
Comment on attachment 173591 [details] [diff] [review]
reimplement |sizeToFit| -- still needs a tweak

If no one's gonna kill my patch, I'll kill it myself :P
Attachment #173591 - Attachment is obsolete: true
Simon, is there a reason the code you checked in does not appear here as a patch?
Yes. a) I'm lazy b) I'm a senior camino developer and can do such things, and c)
you can see the patch in bonsai anyway  :)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: