Tooltip on 'clear' button doesn't show up when the Browser Console is running as a top-level HTML window

RESOLVED FIXED in Firefox 63

Status

defect
P3
normal
RESOLVED FIXED
a year ago
10 months ago

People

(Reporter: bgrins, Assigned: bdahl)

Tracking

Trunk
Firefox 63
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

a year ago
STR:
* ./mach run --setpref devtools.browserconsole.html=true --jsconsole
* Hover the clear messages (trash can) icon in the Browser Console

Expected:
Tooltip text shows up

Actual:
No tooltip text shows up

The hover text does show up when running inside the normal toolbox, I'm assuming because of `this.frame.tooltip = "aHTMLTooltip";` in toolbox-hosts.js.
Priority: -- → P3

Updated

11 months ago
Product: Firefox → DevTools
Reporter

Updated

11 months ago
Depends on: 1469341
Reporter

Updated

11 months ago
Blocks: 1469341
No longer depends on: 1469341
Reporter

Updated

11 months ago
Depends on: 1466897
Comment hidden (mozreview-request)

Comment 2

11 months ago
mozreview-review
Comment on attachment 8989558 [details]
Bug 1460691 - Support tooltips in top level non-XUL windows. +6102

https://reviewboard.mozilla.org/r/254586/#review261472

r+ on the CanvasFrame changes, but please get enndeakin or someone else to review the rest (I'm going on vacation so I don't have time to page that code in)

::: layout/generic/nsCanvasFrame.cpp:341
(Diff revision 1)
>  void
>  nsCanvasFrame::SetDefaultTooltip(Element* aTooltip)
>  {
> -  NS_WARNING("SetDefaultTooltip not implemented");
> -  return;
> +  MOZ_ASSERT(!aTooltip || aTooltip == mTooltipContent,
> +             "Default tooltip should be anonymous content tooltip.");
> +  aTooltip = mTooltipContent;

This assignment should be the other way around.
Attachment #8989558 - Flags: review?(mats) → review+
Assignee

Updated

11 months ago
Assignee: nobody → bdahl
Assignee

Comment 3

11 months ago
Comment on attachment 8989558 [details]
Bug 1460691 - Support tooltips in top level non-XUL windows. +6102

See above^
Attachment #8989558 - Flags: review+ → review?(enndeakin)
Comment hidden (mozreview-request)
Assignee

Updated

11 months ago
Attachment #8989558 - Flags: review?(enndeakin)

Comment 5

11 months ago
mozreview-review
Comment on attachment 8989558 [details]
Bug 1460691 - Support tooltips in top level non-XUL windows. +6102

https://reviewboard.mozilla.org/r/254586/#review263514

::: layout/xul/nsXULTooltipListener.cpp:627
(Diff revision 2)
>        }
>      }
>    }
>  
>  #ifdef MOZ_XUL
> -  // titletips should just use the default tooltip
> +  // titletips and non-XUL documents should just use the default tooltip

The tooltip listener doesn't have any specific checks for XUL elements (most specifically in GetTooltipFor), so if the various attributes it retrieves are not on XUL elements, it would check attributes that shouldn't have any meaning.

Is that desired?
Attachment #8989558 - Flags: review?(enndeakin) → review+
Comment hidden (mozreview-request)
Assignee

Comment 7

11 months ago
Neil,
Just want to make sure you're alright with the new changes^
Flags: needinfo?(enndeakin)
Reporter

Updated

11 months ago
See Also: → 1461793

Comment 8

10 months ago
Pushed by bdahl@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b2d89deed396
Support tooltips in top level non-XUL windows. r=enndeakin+6102,mats+6102
Assignee

Comment 9

10 months ago
I'm going to out of the office the next few weeks, so I went ahead and landed the patch to unblock Brian. Brian mentioned he's fine to do any follow up if there's anything while I'm away.
Flags: needinfo?(enndeakin)

Comment 10

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b2d89deed396
Status: NEW → RESOLVED
Last Resolved: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in before you can comment on or make changes to this bug.