1497544 - No tooltips shown on folder with new messages

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Richard Marti (:Paenglab)]

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).

Comment 1

[Brian Grinstead [:bgrins]]

Are these elements in XUL or HTML documents? And are they in a top level window or included via <browser>/<iframe>?

Comment 2

[Richard Marti (:Paenglab)]

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?

Comment 3

[Richard Marti (:Paenglab)]

Brian, please can you answer my questions in comment 2?

Comment 4

[Brian Grinstead [:bgrins]]

(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.

Comment 5

[Richard Marti (:Paenglab)]

Thank you Brian.

Magnus and Arshad, you are doing XBL conversion. What do you think, what should be done here?

Comment 6

[Brian Grinstead [:bgrins]]

Or maybe the custom tooltips could be implemented with a panel instead?

Comment 7

[Richard Marti (:Paenglab)]

(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?

Comment 8

[Magnus Melin [:mkmelin]]

Sure, upload a patch so we can check it out

Comment 9

[Richard Marti (:Paenglab)]

Attachment: 1497544-imTooltip.patch

This fixes the imTooltips for me.

Comment 10

[Magnus Melin [:mkmelin]]

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?

Comment 11

[Richard Marti (:Paenglab)]

Attachment: 1497544-tooltip.patch v2

This is a Linux only issue. Setting -moz-appearance: tooltip; fixes it.

Comment 12

[Magnus Melin [:mkmelin]]

I still see them with that.

Comment 13

[Richard Marti (:Paenglab)]

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 14

[Magnus Melin [:mkmelin]]

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

Comment 15

[Richard Marti (:Paenglab)]

Thanks. I've set 'leave-open' because of the not fixed folderSummary-popup. Would this be something for Arshad's de-XBL?

Comment 16

[Richard Marti (:Paenglab)]

(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?

Comment 17

[Pulsebot]

Pushed by [e-mail]:
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

Comment 18

[Arshad Khan [:arshad]]

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.

Comment 19

[Richard Marti (:Paenglab)]

Unassigning me, so someone can fix the remaining piece.

Comment 20

[Magnus Melin [:mkmelin]]

I've got this.

Comment 21

[Magnus Melin [:mkmelin]]

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=34e920c2354e45ea82e6fd134ed9eebf95ebc8a0

Comment 22

[Magnus Melin [:mkmelin]]

Attachment: bugXXX_folderSummary.patch

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.)

Comment 23

[Richard Marti (:Paenglab)]

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.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/bd1dfa52d616
de-XBL folderSummary-popup, folderSummary, folderSummary-message. r=Paenglab

Comment 25

[Magnus Melin [:mkmelin]]

Thx. 

Will handle test-tooltip-multimessage.js in bug 1496353.
For SeaMonkey, I filed bug 1504560. Somewhat of a mess.