Closed Bug 1440682 Opened 2 years ago Closed 2 years ago

stylo-chrome: XUL tooltips are not displayed

Categories

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

60 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla60
Tracking Status
firefox-esr52 --- unaffected
firefox58 --- wontfix
firefox59 --- wontfix
firefox60 --- verified

People

(Reporter: roxana.leitan, Assigned: emilio)

References

Details

Attachments

(1 file, 1 obsolete file)

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
Ok, I know why this one happens... Sigh, this XUL code, what a mess...
Assignee: nobody → emilio
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.
Attachment #8953562 - Flags: review?(enndeakin)
Neil, Boris suggested you as a reviewer, but please redirect if you can't get to it or what not. Thanks a lot!
Priority: -- → P1
Summary: The "Show site information" tooltip is not displayed → stylo: The "Show site information" tooltip is not displayed
: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.
Summary: stylo: The "Show site information" tooltip is not displayed → stylo-chrome: XUL tooltips are not displayed
(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 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.
Attachment #8953562 - Flags: review?(enndeakin)
Attachment #8953762 - Flags: review?(enndeakin)
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.
Attachment #8953562 - Attachment is obsolete: true
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.
Attachment #8953762 - Flags: review?(enndeakin) → review+
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.
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e3cce6ae4b15
Make the XUL tooltip stuff saner. r=enn
https://hg.mozilla.org/mozilla-central/rev/e3cce6ae4b15
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
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.
Status: RESOLVED → VERIFIED
Duplicate of this bug: 1440636
Depends on: 1486166
Depends on: 1486957
No longer depends on: 1486957
You need to log in before you can comment on or make changes to this bug.