No tooltips shown on folder with new messages

RESOLVED FIXED in Thunderbird 65.0

Status

defect
RESOLVED FIXED
9 months ago
9 months ago

People

(Reporter: Paenglab, Assigned: mkmelin)

Tracking

({regression})

Trunk
Thunderbird 65.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

When new messages are in the folder hovering it in the folder pane doesn't show the tooltip with the message titles. There are also no tooltips in chat's Conversation- and Participants list since bug 1461798. Backout of this bug fixes the issue (checked only the chat tooltips).
Are these elements in XUL or HTML documents? And are they in a top level window or included via <browser>/<iframe>?
Flags: needinfo?(richard.marti)
They extend the tooltip
https://searchfox.org/comm-central/search?q=xml%23tooltip&case=false&regexp=false&path=
so they don't work because it is now missing. Would it work when we restore the binding and fix this later? Or is also other code removed which is needed?
Flags: needinfo?(richard.marti)
Brian, please can you answer my questions in comment 2?
Flags: needinfo?(bgrinstead)
(In reply to Richard Marti (:Paenglab) from comment #2)
> They extend the tooltip
> https://searchfox.org/comm-central/
> search?q=xml%23tooltip&case=false&regexp=false&path=
> so they don't work because it is now missing. Would it work when we restore
> the binding and fix this later? Or is also other code removed which is
> needed?
 
I'm not sure. I can't think of a technical reason you couldn't restore that binding, although since we're now wiring up the tooltip as a XUL subclass I'm not sure what will happen between the XBL anon content and the NAC added in bug 1461798.

You can try restoring the binding to see if that fixes it for now, but I'm not sure what the right long-term fix for these instances would be since it looks like there is custom DOM inside of them. Maybe either change the UI to use a normal tooltip, or have some kind of JS-implemented widget that uses absolute/fixed positioning? If you see https://bugzilla.mozilla.org/show_bug.cgi?id=1461798#c9 and discussion leading up to it, there was a request to not use Custom Elements inside of the NAC used for tooltips, which is why it's now done with C++ instead.
Flags: needinfo?(bgrinstead)
Thank you Brian.

Magnus and Arshad, you are doing XBL conversion. What do you think, what should be done here?
Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(arshdkhn1)
Or maybe the custom tooltips could be implemented with a panel instead?
(In reply to Brian Grinstead [:bgrins] from comment #6)
> Or maybe the custom tooltips could be implemented with a panel instead?

The ImTooltip works with the panel binding instead the tooltip. The folderSummary-popup doesn't work with it. There is always shown a small empty tooltip/panel.

Magnus, what do you think, should I provide a patch for the ImTooltip using the panel binding and you look for a solution for the folderSummary-popup?
Sure, upload a patch so we can check it out
Flags: needinfo?(mkmelin+mozilla)
Posted patch 1497544-imTooltip.patch (obsolete) — Splinter Review
This fixes the imTooltips for me.
Assignee: nobody → richard.marti
Attachment #9015998 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9015998 [details] [diff] [review]
1497544-imTooltip.patch

Review of attachment 9015998 [details] [diff] [review]:
-----------------------------------------------------------------

It works, but there seems to be some odd black boxes in the background of the tooltip. Not sure what's up with that?
This is a Linux only issue. Setting -moz-appearance: tooltip; fixes it.
Attachment #9015998 - Attachment is obsolete: true
Attachment #9015998 - Flags: review?(mkmelin+mozilla)
Attachment #9016590 - Flags: review?(mkmelin+mozilla)
I still see them with that.
Ah yes. Strange, only when I hover multiple times over the same participant it appears. The first time it's okay. I don't know what this could be.
Comment on attachment 9016590 [details] [diff] [review]
1497544-tooltip.patch v2

Review of attachment 9016590 [details] [diff] [review]:
-----------------------------------------------------------------

Let's go with this then. The visual artefacts can be sorted out later, and with some luck they may sort out themselves. r=mkmelin
Attachment #9016590 - Flags: review?(mkmelin+mozilla) → review+
Thanks. I've set 'leave-open' because of the not fixed folderSummary-popup. Would this be something for Arshad's de-XBL?
(In reply to Richard Marti (:Paenglab) from comment #15)
> Thanks. I've set 'leave-open' because of the not fixed folderSummary-popup.
> Would this be something for Arshad's de-XBL?

Or Magnus, could this be fixed in bug 1498794?
Flags: needinfo?(mkmelin+mozilla)
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/883d9d1734ad
Use popup.xml#panel instead of popup.xml#tooltip for the IMTooltip after the tooltip binding removal. r=mkmelin
Keywords: checkin-needed
Target Milestone: --- → Thunderbird 64.0
Sorry I wasnn't able to respond earlier.. I and Magnus decided in a meeting, that Magnus will take care of this bug. I am ok with whatever Magnus suggests.
Flags: needinfo?(arshdkhn1)
Unassigning me, so someone can fix the remaining piece.
Assignee: richard.marti → nobody
I've got this.
Assignee: nobody → mkmelin+mozilla
Flags: needinfo?(mkmelin+mozilla)
This is getting rid of folderSummary-popup, folderSummary, folderSummary-message, folderSummary-popup, which were strangely intertwined.

With the patch you will see a tooltip summary of new messages (new, not unread), and if there is new mails in a subfolder and the folder is collapsed, it displays an explanation as tooltip.

With mail.biff.use_system_alert;false the notification for new messages is also using this feature.

I also got rid of the black dot tooltip in chat (fillInPageTooltip doesn't exist anymore, but for this case I don't think it was needed either.)
Attachment #9022247 - Flags: review?(richard.marti)
Comment on attachment 9022247 [details] [diff] [review]
bugXXX_folderSummary.patch

Review of attachment 9022247 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks. I like the new notification which add the start of the message content.

With the changes in mailnews, shouldn't the SM guys be informed about this changes?

::: chat/content/imtooltip.xml
@@ +503,5 @@
>           }
>  
>           // Use the default content tooltip.
>           largeTooltip.hidden = true;
> +         return false;

In mail/test/mozmill/folder-display/test-tooltip-multimessage.js is fillInPageTooltip also used. Maybe this can be removed in a new bug.

::: mail/base/content/mailWindowOverlay.js
@@ +1030,5 @@
>  }
>  
> +
> +
> +

For what are this three empty lines?

::: mailnews/base/content/newmailalert.js
@@ +179,5 @@
>    if (gAlertListener)
>      gAlertListener.observe(null, "alertfinished", "");
>    window.close();
>  }
> +

unneeded newline.
Attachment #9022247 - Flags: review?(richard.marti) → review+
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/bd1dfa52d616
de-XBL folderSummary-popup, folderSummary, folderSummary-message. r=Paenglab
Thx. 

Will handle test-tooltip-multimessage.js in bug 1496353.
For SeaMonkey, I filed bug 1504560. Somewhat of a mess.
Status: NEW → RESOLVED
Closed: 9 months ago
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: Thunderbird 64.0 → Thunderbird 65.0
You need to log in before you can comment on or make changes to this bug.