Implement full newsgroup name as tooltip on hover of group name

RESOLVED FIXED in seamonkey2.0b2

Status

SeaMonkey
MailNews: Message Display
--
enhancement
RESOLVED FIXED
15 years ago
8 years ago

People

(Reporter: Marc Schöchlin, Assigned: InvisibleSmiley)

Tracking

Trunk
seamonkey2.0b2
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 6 obsolete attachments)

(Reporter)

Description

15 years ago
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

15 years ago
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
QA Contact: olgam → stephend
Whiteboard: DUPEME

Comment 2

15 years ago
Ok, then we can resolve this with Invalid. There is already an option for it (I
think thats what the Reporter wanted).
Status: UNCONFIRMED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → INVALID
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.
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---
Summary: Display names of newsgroups → Implement full newsgroup name as tooltip on hover of group name
Whiteboard: DUPEME
Confirming as an enhancement request.
Status: UNCONFIRMED → NEW
Ever confirmed: true
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

15 years ago
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

15 years ago
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

14 years ago
*** Bug 210195 has been marked as a duplicate of this bug. ***

Updated

14 years ago
Blocks: 224344

Comment 9

14 years ago
*** Bug 224344 has been marked as a duplicate of this bug. ***

Updated

14 years ago
OS: Linux → All
Hardware: PC → All
works for me on 1.7b build 2004031508 winxp
cool !

Comment 11

14 years ago
(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?
Product: Browser → Seamonkey

Updated

12 years ago
Assignee: sspitzer → mail

Updated

11 years ago
Assignee: mail → nobody
Component: MailNews: Main Mail Window → MailNews: Backend
Product: Mozilla Application Suite → Core
QA Contact: stephend → backend

Comment 12

10 years ago
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

10 years ago
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

10 years ago
Typo: bug 32157 explains the code for the cropped-text tooltip is inline.  Sorry 'bout that.

Updated

9 years ago
Duplicate of this bug: 419165
Product: Core → MailNews Core
I propose making this block Bug 176238.
Is there already a bug on only abbrevating newsgroup names when necessary?
(Assignee)

Comment 18

8 years ago
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?
Component: Backend → MailNews: Message Display
Product: MailNews Core → SeaMonkey
QA Contact: backend → message-display
(Assignee)

Comment 19

8 years ago
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).
Assignee: nobody → jh
Status: NEW → ASSIGNED
Attachment #388579 - Flags: review?(mnyromyr)

Comment 20

8 years ago
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).
Attachment #388579 - Flags: review?(mnyromyr) → review-
(Assignee)

Updated

8 years ago
Blocks: 505056
(Assignee)

Comment 21

8 years ago
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.
Attachment #388579 - Attachment is obsolete: true
Attachment #389332 - Flags: review?(mnyromyr)

Comment 22

8 years ago
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. ;-)
Attachment #389332 - Flags: review?(mnyromyr) → review+
(Assignee)

Updated

8 years ago
Attachment #389332 - Flags: superreview?(neil)
(Assignee)

Comment 23

8 years ago
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).
Are we intending to implement extended toasters too?
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.
(Assignee)

Comment 26

8 years ago
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. :-)
Attachment #389332 - Attachment is obsolete: true
Attachment #392721 - Flags: superreview?(neil)
Attachment #392721 - Flags: review+
Attachment #389332 - Flags: superreview?(neil)
No, toasters are the new mail alerts, apparently...
(Assignee)

Comment 28

8 years ago
(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 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.
Attachment #392721 - Flags: superreview?(neil) → superreview-
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;
(Assignee)

Comment 31

8 years ago
(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).
(Assignee)

Comment 32

8 years ago
Created attachment 394653 [details] [diff] [review]
patch v3

Although now unrelated, I kept the cleanup in parseFolder in this patch.
Attachment #392721 - Attachment is obsolete: true
Attachment #394653 - Flags: superreview?(neil)
Attachment #394653 - Flags: review?(mnyromyr)
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...
(Assignee)

Comment 34

8 years ago
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).
Attachment #394653 - Attachment is obsolete: true
Attachment #394699 - Flags: superreview?(neil)
Attachment #394699 - Flags: review?(mnyromyr)
Attachment #394653 - Flags: superreview?(neil)
Attachment #394653 - Flags: review?(mnyromyr)
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.
Attachment #394699 - Flags: superreview?(neil) → superreview+
(Assignee)

Comment 36

8 years ago
(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...
(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...
(Assignee)

Comment 38

8 years ago
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.
Attachment #394699 - Attachment is obsolete: true
Attachment #394895 - Flags: superreview+
Attachment #394895 - Flags: review?(mnyromyr)
Attachment #394699 - Flags: review?(mnyromyr)

Comment 39

8 years ago
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.
Attachment #394895 - Flags: review?(mnyromyr) → review+
(Assignee)

Comment 40

8 years ago
Created attachment 394914 [details] [diff] [review]
patch v3c
[Checkin: Comment 41]
Attachment #394895 - Attachment is obsolete: true
Attachment #394914 - Flags: superreview+
Attachment #394914 - Flags: review+
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
Attachment #394914 - Attachment description: patch v3c → patch v3c [Checkin: Comment 41]
Comment on attachment 394914 [details] [diff] [review]
patch v3c
[Checkin: Comment 41]


http://hg.mozilla.org/comm-central/rev/c1d838bb29a7
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0b2
You need to log in before you can comment on or make changes to this bug.