Last Comment Bug 174234 - Implement full newsgroup name as tooltip on hover of group name
: Implement full newsgroup name as tooltip on hover of group name
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Message Display (show other bugs)
: Trunk
: All All
: -- enhancement with 14 votes (vote)
: seamonkey2.0b2
Assigned To: Jens Hatlak (:InvisibleSmiley)
:
Mentors:
: 210195 224344 419165 (view as bug list)
Depends on:
Blocks: 224344 505056
  Show dependency treegraph
 
Reported: 2002-10-13 09:56 PDT by Marc Schöchlin
Modified: 2009-08-20 06:10 PDT (History)
17 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
proposed patch (7.04 KB, patch)
2009-07-14 15:23 PDT, Jens Hatlak (:InvisibleSmiley)
mnyromyr: review-
Details | Diff | Splinter Review
patch v2 (7.15 KB, patch)
2009-07-18 16:43 PDT, Jens Hatlak (:InvisibleSmiley)
mnyromyr: review+
Details | Diff | Splinter Review
patch v2a, r=mnyromyr (7.20 KB, patch)
2009-08-05 08:08 PDT, Jens Hatlak (:InvisibleSmiley)
jh: review+
neil: superreview-
Details | Diff | Splinter Review
patch v3 (8.71 KB, patch)
2009-08-15 07:44 PDT, Jens Hatlak (:InvisibleSmiley)
no flags Details | Diff | Splinter Review
patch v3a (8.76 KB, patch)
2009-08-16 01:21 PDT, Jens Hatlak (:InvisibleSmiley)
neil: superreview+
Details | Diff | Splinter Review
patch v3b (8.78 KB, patch)
2009-08-17 14:24 PDT, Jens Hatlak (:InvisibleSmiley)
mnyromyr: review+
jh: superreview+
Details | Diff | Splinter Review
patch v3c [Checkin: Comment 41] (8.79 KB, patch)
2009-08-17 15:08 PDT, Jens Hatlak (:InvisibleSmiley)
jh: review+
jh: superreview+
Details | Diff | Splinter Review

Description Marc Schöchlin 2002-10-13 09:56:19 PDT
Hi !

Because newsgroupnames are regulary very long - mozilla shortens this
names to something like this: 

n.p.n.whishlist

It would be very nice, if the complete name will be shown if 
the mousepinter remains longer than a half second over this shortend
name.

If I have a lot of similar newsgroups - it is hard
to differentiate them.

Regards

Marc Schoechlin
Comment 1 tloehnert 2002-10-13 10:43:19 PDT
independent from your suggestion for an enhancement: there is a way to have
mozilla show the unabbreviated newsgroup name
http://www.mozilla.org/start/1.0/faq/mail-news.html#3.14
Comment 2 Frank Wein [:mcsmurf] 2002-10-27 08:49:50 PST
Ok, then we can resolve this with Invalid. There is already an option for it (I
think thats what the Reporter wanted).
Comment 3 stephend@netscape.com (gone - use stephen.donner@gmail.com instead) 2002-11-04 16:51:48 PST
See bug 41613, bug 61795 and bug 61794

Also, bug 32157 deals with tooltips over cropped XUL-elements (any text, basically).

Since those don't deal specifically with expanding the tooltip for a shortened
name, and none of them have been implemented yet (to take a look at), I'll
reopen this and let it stand.

Adjusting summary to "Implement full newsgroup name as tooltip on hover of group
name" to be more precise.
Comment 4 stephend@netscape.com (gone - use stephen.donner@gmail.com instead) 2002-11-04 16:54:07 PST
Confirming as an enhancement request.
Comment 5 http://tinymailto.com/oliversl 2002-11-18 11:50:21 PST
The tooltip will help for example in this real world case:
p.p.p.general -> powersoft.public.powerdesigner.general
p.p.p.general -> powersoft.public.powerdesigner.general

The real name of the newsgroup is so long that I choose to use the short name
option. But since both groups have the save short path, I have to click on th
groups and look at the MailNews window title to fin out in wich group I'm.

Since the long name of the group is displayed in the window title, maybe that
string can be asigned to the tooltip.

Comment 6 Ant 2003-02-09 17:45:00 PST
Just a note. I am voting this and adding my two cents. All ports of platforms
(used PowerBook G4 and PCs) and OS' (Windows 98, XP Professional, Windows 2000,
and Red Hat Linux v7.x) seem to have this annoyance. I would love to see a
tooltip as well. I do not want to turn off abbreviation and waste my precious
screen real estate. :)
Comment 7 Rob Yampolsky 2003-02-10 09:53:24 PST
Not only should there be a tooltip on hover over group names, but it would be
very nice to do it on hover over any column in the article list where the text
is too long to display.  Resizing the Subject column to view a long article name
ends up shrinking some other column.  There's only so much screen space
available, and the user should be able to size the columns as they like but
still be able to see things that don't fit.

NS4 does this, Outlook does this, Mozilla should too.  For me, it's the only
'step backward' that I have to accept in moving to Mozilla.
Comment 8 Frederic Bezies 2003-06-21 05:39:04 PDT
*** Bug 210195 has been marked as a duplicate of this bug. ***
Comment 9 Benedikt Kantus 2003-11-01 03:41:47 PST
*** Bug 224344 has been marked as a duplicate of this bug. ***
Comment 10 Olivier Vit (just a reporter) 2004-03-16 08:36:22 PST
works for me on 1.7b build 2004031508 winxp
cool !
Comment 11 Vaclav Dvorak 2004-03-16 11:28:25 PST
(In reply to comment #10)
> works for me on 1.7b build 2004031508 winxp

I don't think so. Doesn't WFM on Linux with a build about an hour old.

This bug is specifically about newsgroup names. Perhaps you mean bug 32157?
Comment 12 steg 2007-04-29 13:18:56 PDT
Doesn't work for me either on Windows Millennium with SeaMonkey 1.1.1 (Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.8.1.2) Gecko/20070222 SeaMonkey/1.1.1) nor with Thunderbird 2.0 (version 2.0.0.0 (20070326)).
Comment 13 Lucas Yamanishi 2008-01-16 04:55:15 PST
I tried to force a workaround using tooltiptext on tree id folderTree (line 147) in messenger.xul, but I can't get the variables or datasources to work.  I tried "rdf:http://home.netscape.com/NC-rdf#FolderTreeSimpleName" and "?folderTreeSimpleName" as values, both of which are used to describe the actual folder cell labels.  The datasources value for the tree is set to rdf:null, so I'm wondering if there is something I need to add there...  I'm very new to XUL and JavaScript, so any help here is appreciated.

The result I did experienced was an empty tooltip box on hover over non-cropped folders, but an abbreviated name and unreadCount on cropped folder names, as would be expected.  The weird thing was that the value of the cropped folder name persisted and subsequently appeared for un-cropped folders. Bug 204786 explains the code for the cropped-text tooltip is a workaround built into the engine, so I'm wondering if maybe that is interfering with the values I tried to input.  Again, I'm lacking experiance and C is a little too much for me to tackle right now-- help is appreciated.

Also of interest, since it might be desirable to make folder names titletips (tooltips without a delay), bug 204786 to add a delay attribute to the tooltip element, and bug 45079 (line 273, am-server.xul) dealing with the abbreviation settings.
Comment 14 Lucas Yamanishi 2008-01-16 05:04:27 PST
Typo: bug 32157 explains the code for the cropped-text tooltip is inline.  Sorry 'bout that.
Comment 15 zug_treno 2008-03-11 08:46:26 PDT
*** Bug 419165 has been marked as a duplicate of this bug. ***
Comment 16 Jens Müller (:tessarakt) 2008-10-22 11:13:16 PDT
I propose making this block Bug 176238.
Comment 17 Jens Müller (:tessarakt) 2008-11-03 11:10:58 PST
Is there already a bug on only abbrevating newsgroup names when necessary?
Comment 18 Jens Hatlak (:InvisibleSmiley) 2009-07-06 14:11:36 PDT
This was just fixed for TB (bug 271841). Comparing our/their mailWidgets.xml and looking at the code changed by their patch it should be easy to port, maybe even 1:1 (including the line wrap changes). Lucas, do you want to give it a try?
Comment 19 Jens Hatlak (:InvisibleSmiley) 2009-07-14 15:23:50 PDT
Created attachment 388579 [details] [diff] [review]
proposed patch

Ported bug 271841 plus the necessary changes from TB. Maybe not all CSS rules are needed but the bindings are already there (in mailWidgets.xml) so I copied all three.

Note: IMO this bug should not morph into implementing the SM part of folder previews. I know code parts are already there but the anticipated changes needed (CSS styles for different themes/platforms etc.) should be enough to justify an own bug (if anyone wants to file one: TB bug was bug 314124).
Comment 20 Karsten Düsterloh 2009-07-18 06:24:09 PDT
Comment on attachment 388579 [details] [diff] [review]
proposed patch

Tooltips for abbreviated groups work, but the patch has issues.

>diff --git a/suite/mailnews/mailWidgets.xml b/suite/mailnews/mailWidgets.xml
>+            // Use the full newsgroup name as tooltip for abbreviated newsgroups.
>+            if ((aFolder.server instanceof Components.interfaces.nsINntpIncomingServer) &&
>+                !(aFolder.flags & Components.interfaces.nsMsgFolderFlags.Virtual) &&
>+                aFolder.server.abbreviate) {

You're using aFolder.server before checking for !aFolder.
And curly braces should be on their own line in SM mailnews land.

>+              var msgPopup = document.createElementNS("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul", "label");

You should use 'let' for subscope variables. 

>             var showPreviewText = pref.getBoolPref("mail.biff.alert.show_preview");

This results in 

Error: uncaught exception: [Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getBoolPref]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: chrome://messenger/content/mailWidgets.xml :: parseFolder :: line 2044"  data: no]

plus a small empty tooltip for any non-newsgroup, non-server folders, because this pref doesn't exist in SM land yet. It should have a default value now we're using it.
(And we should implement the rest of the preview stuff, else this code wouldn't make sense anyway.)

>-              var RDF = Components.classes['@mozilla.org/rdf/rdf-service;1'].getService().QueryInterface(Components.interfaces.nsIRDFService);
>+              var RDF = Components.classes['@mozilla.org/rdf/rdf-service;1']
>+                                  .getService()
>+                                  .QueryInterface(Components.interfaces.nsIRDFService);

Use getService(Components.interfaces.nsIRDFService).
Comment 21 Jens Hatlak (:InvisibleSmiley) 2009-07-18 16:43:44 PDT
Created attachment 389332 [details] [diff] [review]
patch v2

(In reply to comment #20)
> >             var showPreviewText = pref.getBoolPref("mail.biff.alert.show_preview");
> 
> This results in 
> 
> Error: uncaught exception: [Exception... "Component returned failure code:
> 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getBoolPref]"  nsresult:
> "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame ::
> chrome://messenger/content/mailWidgets.xml :: parseFolder :: line 2044"  data:
> no]
> 
> plus a small empty tooltip for any non-newsgroup, non-server folders, because
> this pref doesn't exist in SM land yet. It should have a default value now
> we're using it.

The tooltip is also empty if all three prefs are present but set to FALSE (same in TB). None of those is actually needed for this bug, though; they just define which information should be shown in a non-NG, non-account folder with NEW (unseen) messages. I'd say spare that for the other bug and just disable the tooltip for those cases for the time being, see new patch.

> (And we should implement the rest of the preview stuff, else this code wouldn't
> make sense anyway.)

Filed bug 505056, depending on this one.
Comment 22 Karsten Düsterloh 2009-07-21 11:39:52 PDT
Comment on attachment 389332 [details] [diff] [review]
patch v2

>diff --git a/suite/mailnews/mailWidgets.xml b/suite/mailnews/mailWidgets.xml
>+            // Disable extended alerts (sender/subject/preview) until implemented (bug 505056)
>+            return false;

Well, that's a valid possibility as well. ;-)
Comment 23 Jens Hatlak (:InvisibleSmiley) 2009-07-21 11:50:26 PDT
Comment on attachment 389332 [details] [diff] [review]
patch v2

(In reply to comment #22)
> (From update of attachment 389332 [details] [diff] [review])
> >diff --git a/suite/mailnews/mailWidgets.xml b/suite/mailnews/mailWidgets.xml
> >+            // Disable extended alerts (sender/subject/preview) until implemented (bug 505056)
> >+            return false;
> 
> Well, that's a valid possibility as well. ;-)

Yes, and in bug 505056 this should be replaced by a check against all three mail.biff.alert.* prefs (if all three are false, return false).
Comment 24 neil@parkwaycc.co.uk 2009-08-05 05:45:36 PDT
Are we intending to implement extended toasters too?
Comment 25 neil@parkwaycc.co.uk 2009-08-05 05:50:38 PDT
Comment on attachment 389332 [details] [diff] [review]
patch v2

>+              let msgPopup = document.createElementNS("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul", "label");
>+              msgPopup.setAttribute("value", aFolder.name);
Assuming the above is true, then (I wasn't able to test the patch, it wouldn't apply to my build; I guess I've taken too long to get around to reviewing, sorry) the only thing I have a problem with is that this label probably needs a className of "tooltip-label" so that popup.css can apply styling to it.
Comment 26 Jens Hatlak (:InvisibleSmiley) 2009-08-05 08:08:45 PDT
Created attachment 392721 [details] [diff] [review]
patch v2a, r=mnyromyr

(In reply to comment #24)
> Are we intending to implement extended toasters too?

Come again? You mean whether we should do bug 505056?

(In reply to comment #25)
> (From update of attachment 389332 [details] [diff] [review])
> >+              let msgPopup = document.createElementNS("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul", "label");
> >+              msgPopup.setAttribute("value", aFolder.name);
> Assuming the above is true, then (I wasn't able to test the patch, it wouldn't
> apply to my build; I guess I've taken too long to get around to reviewing,
> sorry) the only thing I have a problem with is that this label probably needs a
> className of "tooltip-label" so that popup.css can apply styling to it.

Ah, yes, now without margin. :-)
Comment 27 neil@parkwaycc.co.uk 2009-08-05 08:28:02 PDT
No, toasters are the new mail alerts, apparently...
Comment 28 Jens Hatlak (:InvisibleSmiley) 2009-08-05 09:53:15 PDT
(In reply to comment #27)
> No, toasters are the new mail alerts, apparently...

Ah OK. Well, those deserve an own bug I guess, especially since the styling will probably be platform-dependent. I didn't check if one exists already and won't file one either for now since I'd want to know in which bug they were developed first (to form a nice "Port bug xxxxxx - ..." summary). Also I guess that one might lead into C++ land where I don't exactly feel at home (maybe I'm wrong, maybe that's shared code that doesn't need any changes from our side).
Comment 29 neil@parkwaycc.co.uk 2009-08-10 07:42:53 PDT
Comment on attachment 392721 [details] [diff] [review]
patch v2a, r=mnyromyr

In case we do use extended toasters, we'll need this code for them, so we can't change it here. Instead we'll need to change the caller (folderSummary-popup binding). Another problem with this code is that it disables the default tree tooltip, so you get no tooltips on cropped long mail folder names. Oh, and one final thing: when the Unread column is hidden then the Name column shows the unread count (if nonzero) too.
Comment 30 neil@parkwaycc.co.uk 2009-08-10 07:46:39 PDT
FYI the "default tree tooltip" code works something like this:

var row = {};
var col = {};
var obj = {};
var b = GetFolderTree().treeBoxObject;
b.getCellAt(event.clientX, event.clientY, row, col, obj);
if (!b.isCellCropped(row, col))
  return false;
this.label.value = GetFolderTree().view.getCellText(row, col);
return true;
Comment 31 Jens Hatlak (:InvisibleSmiley) 2009-08-10 13:35:50 PDT
(In reply to comment #29)
> (From update of attachment 392721 [details] [diff] [review])
> In case we do use extended toasters, we'll need this code for them, so we can't
> change it here. Instead we'll need to change the caller (folderSummary-popup
> binding).

What I'm doing here is basically a port of Thunderbird bug 271841 (see comment 18). If it worked for them, how could it not work for us? They already have "toasters"!

Maybe I should read your "can't" as "shouldn't" here?

> Another problem with this code is that it disables the default tree
> tooltip, so you get no tooltips on cropped long mail folder names.

True. The TB guys seem to have overlooked that, good catch! Also, thanks for the hint (comment 30).
Comment 32 Jens Hatlak (:InvisibleSmiley) 2009-08-15 07:44:17 PDT
Created attachment 394653 [details] [diff] [review]
patch v3

Although now unrelated, I kept the cleanup in parseFolder in this patch.
Comment 33 neil@parkwaycc.co.uk 2009-08-15 14:32:49 PDT
Comment on attachment 394653 [details] [diff] [review]
patch v3

>+          var row = {};
>+          var col = {};
>+          var childElt = {};
>+          folderTree.treeBoxObject.getCellAt(event.clientX, event.clientY,
>+                                             row, col, childElt);
>+          var row = row.value;
>+          var col = col.value;
Nit: don't need to declare these again, just assign ;-)

>           var msgFolder = GetFolderResource(folderTree, row).QueryInterface(Components.interfaces.nsIMsgFolder);
>           if (!msgFolder || msgFolder.isServer)
>             return false;
!msgFolder will never be true, and we don't want to bail out for servers yet.

>+          // Use the full newsgroup name as tooltip for abbreviated newsgroups.
>+          if ((msgFolder.server instanceof Components.interfaces.nsINntpIncomingServer) &&
>+              !(msgFolder.flags & Components.interfaces.nsMsgFolderFlags.Virtual) &&
>+              msgFolder.server.abbreviate)
But we don't want to do this for servers, and we only want to do this for the Name column.

>+            let msgPopup = document.createElementNS("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul", "label");
Silly name for this variable.

>+          // Disable extended alerts (sender/subject/preview) until implemented (bug 505056)
Hmm, we have to decide when to show an extended alert and when to show a cropped cell tooltip...
Comment 34 Jens Hatlak (:InvisibleSmiley) 2009-08-16 01:21:24 PDT
Created attachment 394699 [details] [diff] [review]
patch v3a

(In reply to comment #33)
> >           var msgFolder = GetFolderResource(folderTree, row).QueryInterface(Components.interfaces.nsIMsgFolder);
> >           if (!msgFolder || msgFolder.isServer)
> >             return false;
> !msgFolder will never be true, and we don't want to bail out for servers yet.

That code part was introduced in bug 314124. You're right about servers (we should show a tooltip for these as well if the cell is cropped) but I'm not sure about !msgFolder so I'll just take your word for it.

I removed both lines, the checks are also done at the beginning of parseFolder.

> >+          // Disable extended alerts (sender/subject/preview) until implemented (bug 505056)
> Hmm, we have to decide when to show an extended alert and when to show a
> cropped cell tooltip...

Yes, but not here. ;-) Probably something like "if parseFolder adds nothing, show cropped cell tooltip". But there's more to do there anyway: if all three extended alert prefs are set to false, parseFolder adds an empty msgPopup tooltip (even in current Shredder builds). parseFolder should probably just return false in that case. We'd have to double check that that is OK for all future cases, though (AFAIK Shredder uses parseFolder both for tooltips and toasters).
Comment 35 neil@parkwaycc.co.uk 2009-08-16 05:54:39 PDT
Comment on attachment 394699 [details] [diff] [review]
patch v3a

>+          row = row.value;
>+          col = col.value;
>+
>           if (row == -1)
>             return false;
>           var msgFolder = GetFolderResource(folderTree, row).QueryInterface(Components.interfaces.nsIMsgFolder);
>+
>+          var popupValue = null;
>+
I'm not sure why you put these three blank lines in, as none of them were in the one place that I would have expected (which was just after the return false;) but I don't know which blank lines Mnyromyr might want.

>+              (msgFolder.server instanceof Components.interfaces.nsINntpIncomingServer) &&
(msgFolder instanceof Components.interfaces.nsIMsgNewsFolder) might also work.
Comment 36 Jens Hatlak (:InvisibleSmiley) 2009-08-16 09:46:45 PDT
(In reply to comment #35)
> (From update of attachment 394699 [details] [diff] [review])
> >+          row = row.value;
> >+          col = col.value;
> >+
> >           if (row == -1)
> >             return false;
> >           var msgFolder = GetFolderResource(folderTree, row).QueryInterface(Components.interfaces.nsIMsgFolder);
> >+
> >+          var popupValue = null;
> >+
> I'm not sure why you put these three blank lines in, as none of them were in
> the one place that I would have expected (which was just after the return
> false;) but I don't know which blank lines Mnyromyr might want.

This is what I have locally now:

          folderTree.treeBoxObject.getCellAt(event.clientX, event.clientY,
                                             row, col, childElt);
          [row, col] = [row.value, col.value];
          if (row == -1)
            return false;

          var msgFolder = GetFolderResource(folderTree, row).QueryInterface(Components.interfaces.nsIMsgFolder);
          var popupValue = null;
          // Use the full newsgroup name as tooltip for abbreviated newsgroups.

The alternate row/col swapping code was an idea Ratty had on IRC.

> >+              (msgFolder.server instanceof Components.interfaces.nsINntpIncomingServer) &&
> (msgFolder instanceof Components.interfaces.nsIMsgNewsFolder) might also work.

It doesn't, but please don't ask me why...
Comment 37 neil@parkwaycc.co.uk 2009-08-17 08:47:56 PDT
(In reply to comment #36)
>           [row, col] = [row.value, col.value];
No, thank you.

> > >+              (msgFolder.server instanceof Components.interfaces.nsINntpIncomingServer) &&
> > (msgFolder instanceof Components.interfaces.nsIMsgNewsFolder) might also work.
> It doesn't, but please don't ask me why...
Strange, I'm sure it worked when I tried...
Comment 38 Jens Hatlak (:InvisibleSmiley) 2009-08-17 14:24:56 PDT
Created attachment 394895 [details] [diff] [review]
patch v3b

(In reply to comment #37)
> > > >+              (msgFolder.server instanceof Components.interfaces.nsINntpIncomingServer) &&
> > > (msgFolder instanceof Components.interfaces.nsIMsgNewsFolder) might also work.
> > It doesn't, but please don't ask me why...
> Strange, I'm sure it worked when I tried...

Short summary for those not on IRC: the line itself would work but the check against nsINntpIncomingServer it would replace is needed for the server.abbreviate check to work.
Comment 39 Karsten Düsterloh 2009-08-17 15:01:54 PDT
Comment on attachment 394895 [details] [diff] [review]
patch v3b

>+          folderTree.treeBoxObject.getCellAt(event.clientX, event.clientY,
>+                                             row, col, childElt);

Not wrapping this is permissable for enhanced readability.

>+          // Use the full newsgroup name as tooltip for abbreviated newsgroups.
>+          if (col.id == "folderNameCol" && !msgFolder.isServer &&
>+              (server instanceof Components.interfaces.nsINntpIncomingServer) &&
>+              !(msgFolder.flags & Components.interfaces.nsMsgFolderFlags.Virtual) &&
>+              server.abbreviate)

Move the "(server instanceof..." one line down, so that the QI to nsINntpIncomingServer only happens when needed.

>+            popupValue = msgFolder.name;
>+          // Show full cell content as tooltip for cropped cells.
>+          else if (folderTree.treeBoxObject.isCellCropped(row, col))
>+            popupValue = folderTree.view.getCellText(row, col);

Furthermore, for enhanced readability, put both branches into {}.

r=me with that.
Comment 40 Jens Hatlak (:InvisibleSmiley) 2009-08-17 15:08:13 PDT
Created attachment 394914 [details] [diff] [review]
patch v3c
[Checkin: Comment 41]
Comment 41 Serge Gautherie (:sgautherie) 2009-08-20 06:09:44 PDT
Comment on attachment 394914 [details] [diff] [review]
patch v3c
[Checkin: Comment 41]


http://hg.mozilla.org/comm-central/rev/c1d838bb29a7

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