1440682 - stylo-chrome: XUL tooltips are not displayed

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect, P1)

Description

[roxleitan]

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0
Build ID:20180223100113

[Affected versions]:
Nightly 60.0a1

[Affected platforms]:
Ubuntu 16.04 X64, Windows 10 x64, Mac OS X 10.13

[Steps to reproduce]:
1.Launch Nightly 60.0a1 with a new profile
2.Go to a website (e.g. http://permission.site)
3.Hover using the mouse over "i" from the address bar

[Expected result]:
The tooltip should be displayed

[Actual result]:
The tooltip is not displayed

[Note]:
The issue is not reproducible with layout.css.chrome.enabled=false

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Ok, I know why this one happens... Sigh, this XUL code, what a mess...

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Make the XUL tooltip stuff saner.

So much wrong stuff with this one.

For starters, we never removed the event listeners (the code was there, lol, but
the function that was supposed to call into the tooltip listener returned
NS_ERROR_NOT_IMPLEMENTED instead).

Furthermore, we added an event listener each time we reframed an element, which
is insane. Basically, each time an element with tooltip / tooltiptext gets its
frame tree reconstructed, we add the even listener, again, and we never free it.

Move the code from the RestyleManager and the frame constructor to AfterSetAttr
/ BindToTree / UnbindFromTree in nsXULElement to hopefully make this saner.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Neil, Boris suggested you as a reviewer, but please redirect if you can't get to it or what not. Thanks a lot!

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

:cpeterson noticed that the DevTools tab and button tooltips don't appear (which use XUL).  I am assuming this is the same issue, but please revert my bug changes if I am incorrect.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #4)
> :cpeterson noticed that the DevTools tab and button tooltips don't appear
> (which use XUL).  I am assuming this is the same issue, but please revert my
> bug changes if I am incorrect.

Yes, it's the same.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8953562 [details] [diff] [review]
Make the XUL tooltip stuff saner.

Hmm, this has some orange because of UnbindFromTree running from cycle collection... Will figure out what the right thing to do is there.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Make the XUL tooltip stuff saner.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8953562 [details] [diff] [review]
Make the XUL tooltip stuff saner.

That version should be better. RemoveEventListener can be the last thing we do, so we need to hold on a reference.

Ideally the nsXULTooltipListener would just be a StaticRefPtr<> and we'd just call ClearOnShutDown, but since it can hold a strong ref to a tree column, I suspect that could leak very heavily, so I did the easiest and held a reference from RemoveTooltipSupport instead.

Comment 9

[Neil Deakin]

Comment on attachment 8953762 [details] [diff] [review]
Make the XUL tooltip stuff saner.

>+  if (aXULElement.NodeInfo()->Equals(nsGkAtoms::treechildren)) {
>+    // treechildren always get tooltip support, apparently.
>+    return true;

This is because cropped tree cells show their full text in a tooltip.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Thanks for the review!

(In reply to Neil Deakin from comment #9)
> Comment on attachment 8953762 [details] [diff] [review]
> Make the XUL tooltip stuff saner.
> 
> >+  if (aXULElement.NodeInfo()->Equals(nsGkAtoms::treechildren)) {
> >+    // treechildren always get tooltip support, apparently.
> >+    return true;
> 
> This is because cropped tree cells show their full text in a tooltip.

I'll note this in a comment before landing.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e3cce6ae4b15
Make the XUL tooltip stuff saner. r=enn

Comment 12

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/e3cce6ae4b15

Comment 13

[Hani Yacoub, Desktop QA]

Build ID: 20180228100110
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0

Verified as fixed on Firefox Nightly 60.0a1 on Windows 10 x 64, Windows 7 x32, Mac OS X 10.12 and Ubuntu 16.04 x64.