1285206 - HTMLTooltip scroll is not working when using a XUL Panel wrapper

Status: VERIFIED FIXED

(DevTools :: Shared Components, defect, P1)

Description

[Julian Descottes [:jdescottes]]

Follow up to Bug 1267403.

Context:
Devtools are now using a HTML implementation of a tooltip, which is supposed to replace the XUL panel for devtools. However, when hosted in a XUL document we try to use a XUL panel as a container for the HTMLTooltip, in order to allow the tooltip to extend beyond the devtools toolbox.

Issue:
When using a HTMLTooltip relying on a xulPanelWrapper, scroll will stop working unless the mouse cursor is on the XUL panel.

Setup:
When bug 1285189 lands, HTMLTooltips will no longer use a XUL panel wrapper by default. To reproduce the issue, you need to first enable it at https://hg.mozilla.org/mozilla-central/file/4764b9f8e6d4/devtools/client/shared/widgets/HTMLTooltip.js#l203

STRs:
- go to https://www.mozilla.org (for instance)
- open devtools/inspector
- expand markup nodes in order to have a scrollbar in the markup view
- click on the "ev" bubble next to a node in the markup view
- (event details tooltip should open)
- make sure the mouse is not on the tooltip (some parts of the tooltip are invisible, to be sure, you can keep the mouse on the "ev" element)
- try to scroll

ER: markup view should scroll
AR: nothing happens

Comment 1

[Julian Descottes [:jdescottes]]

Hi Neil, (have a look at the summary to get some context, ping me if you need more info)

We are trying to use XUL panels as a wrapper to allow us to position HTML elements anywhere on the screen, but with our current panel configuration, scroll events are prevented as soon as the panel gets displayed.

The panel is created at https://hg.mozilla.org/mozilla-central/file/4764b9f8e6d4/devtools/client/shared/widgets/HTMLTooltip.js#l536 and I will just list the attributes here for reference:

>   let panel = this.doc.createElementNS(XUL_NS, "panel");
>   panel.setAttribute("animate", false);
>   panel.setAttribute("consumeoutsideclicks", false);
>   panel.setAttribute("noautofocus", true);
>   panel.setAttribute("ignorekeys", true);
>   panel.setAttribute("level", "float");
>   panel.setAttribute("class", "tooltip-xul-wrapper");

From what I tested, this issue goes away when I set the "type" to "arrow". This is not ideal, because I don't want the XUL panel to have any kind of visible decoration. I can of course remove them with CSS, but it looks like a hacky workaround.

Another solution seems to be to set "noautohide" to true. But this one seems to elevate the panel to a different type of window, and on Ubuntu for instance, the panel becomes resizable when setting this parameter (did not find any way to prevent this).

Ideally I would like to have the scroll behavior I get when setting the "arrow" type, but without any of the extra markup/css.

Could you give us some advice here? Is there a way to enable scroll events with the current panel implementation? (Or maybe this is just a very bad idea ?)

Comment 2

[Neil Deakin]

Is this actually a tooltip? If so, you <tooltip>. It doesn't sound like it actually is a tooltip though. If you need to click to open it then it isn't a tooltip.

If it is a regular sort of menu or popup that you are expected to click to open, then you shouldn't be able to scroll outside the popup while it is open.

Comment 3

[Julian Descottes [:jdescottes]]

(In reply to Neil Deakin from comment #2)
> Is this actually a tooltip? If so, you <tooltip>. It doesn't sound like it
> actually is a tooltip though. If you need to click to open it then it isn't
> a tooltip.

By tooltip here, I encapsulate the various tooltips/panels/popups we use in devtools today. Some of them open on click, some on hover, some automatically, some are interactive, some are not. But all were using a XUL panel. We are migrating the devtools to use HTML instead of XUL so we are migrating all our panels from using XUL <panel> elements to use HTML elements. 

One of the shortcomings of this migration is that our HTML elements have to live in the same document as the devtools toolbox, which can be quite small. Our tooltips were designed with a XUL panel in mind, ie. they could use all the screen. As a result they are really big.

So in order to make the transition easier, we thought we could try to use a XUL panel to wrap our HTML Tooltips, just in order to have access to the rest of the screen.

> 
> If it is a regular sort of menu or popup that you are expected to click to
> open, then you shouldn't be able to scroll outside the popup while it is
> open.

In FF49 or older, the event details tooltip used in devtools is using a <panel type="arrow">, opens on click but still allows scrolling. So it seems this behavior is supported by the current XUL panel, just not with default type.

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1285206 - Use type=arrow for xul panel wrapper in HTMLTooltip;

Review commit: https://reviewboard.mozilla.org/r/63052/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/63052/

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: Bug 1285206 - Enable xul panel for some devtools tooltips;

Set useXulWrapper to true for markup view image previews and rule view
tooltips.

Also slightly changed the logic in HTMLTooltip.js so that useXulWrapper is only
true when we are in a XUL context.

Review commit: https://reviewboard.mozilla.org/r/63160/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/63160/

Comment 6

[Julian Descottes [:jdescottes]]

Comment on attachment 8769079 [details]
Bug 1285206 - Use type=arrow for xul panel wrapper in HTMLTooltip;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63052/diff/1-2/

Comment 7

[Neil Deakin]

> Some of them open on click, some on hover, some automatically, some are interactive, some are not.

It sounds like you seen to think that all of these should behave the same in this regard but they do not. The specific type of popup is used to determine whether scrolling is allowed, scrolling is not allowed, or the popup is hidden when scrolling occurs and is designed to match the operating system behaviour.

Comment 8

[Julian Descottes [:jdescottes]]

try : https://treeherder.mozilla.org/#/jobs?repo=try&revision=9325d6b7a1efc9bd5e07c04781b1076751d0594d

Comment 9

[Brian Grinstead [:bgrins]]

https://reviewboard.mozilla.org/r/63052/#review60392

::: devtools/client/themes/tooltips.css:146
(Diff revision 2)
>    display: flex;
>    visibility: hidden;
>  }
>  
> +/* type="arrow" overrides: remove arrow decorations for the xul <panel> wrapper */
> +/* Arrow element does not need to be hidden because the panel is absolutely positioned */

Might it be easier to just display: none on the arrow?  Especially if there's platform-specific styles that need to be overridden

Comment 10

[Brian Grinstead [:bgrins]]

Comment on attachment 8769189 [details]
Bug 1285206 - Enable xul panel for some devtools tooltips;

https://reviewboard.mozilla.org/r/63160/#review60394

Comment 11

[Julian Descottes [:jdescottes]]

Comment on attachment 8769079 [details]
Bug 1285206 - Use type=arrow for xul panel wrapper in HTMLTooltip;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63052/diff/2-3/

Comment 12

[Julian Descottes [:jdescottes]]

Comment on attachment 8769189 [details]
Bug 1285206 - Enable xul panel for some devtools tooltips;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63160/diff/1-2/

Comment 13

[Julian Descottes [:jdescottes]]

https://reviewboard.mozilla.org/r/63052/#review60392

> Might it be easier to just display: none on the arrow?  Especially if there's platform-specific styles that need to be overridden

I updated this to set display none on the XUL arrow container, but I realize my comment might have been confusing. 

"Arrow element does not need to be hidden because the panel is absolutely positioned". What this actually meant is that the arrow "image" element already has hidden set to true, because the panel is positioned using openPopupAtScreen(). Since it doesn't have any anchor, the XUL panel is hiding the arrow.

Still doesn't hurt to enforce display none on the arrow container. Also updated my comment.

Comment 14

[Brian Grinstead [:bgrins]]

Comment on attachment 8769079 [details]
Bug 1285206 - Use type=arrow for xul panel wrapper in HTMLTooltip;

https://reviewboard.mozilla.org/r/63052/#review60666

::: devtools/client/themes/tooltips.css:153
(Diff revision 3)
> +  margin: 0;
> +}
> +
> +/* The actual XUL arrow image is hidden because the panel is opened using
> +openPopupAtScreen(), additionally set display: none on the arrow container. */
> +.tooltip-xul-wrapper[type="arrow"] .panel-arrowbox {

We discussed on irc about this - this block of code can be removed since it's hidden in the markup

Comment 15

[Julian Descottes [:jdescottes]]

Comment on attachment 8769079 [details]
Bug 1285206 - Use type=arrow for xul panel wrapper in HTMLTooltip;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63052/diff/3-4/

Comment 16

[Julian Descottes [:jdescottes]]

Comment on attachment 8769189 [details]
Bug 1285206 - Enable xul panel for some devtools tooltips;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63160/diff/2-3/

Comment 17

[Julian Descottes [:jdescottes]]

Thanks for the reviews! Rebased and removed the useless display: none;

Pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c8e2110d67a7

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/c368e6e3c0f7
Use type=arrow for xul panel wrapper in HTMLTooltip;r=bgrins
https://hg.mozilla.org/integration/fx-team/rev/55922a4a546c
Enable xul panel for some devtools tooltips;r=bgrins

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c368e6e3c0f7
https://hg.mozilla.org/mozilla-central/rev/55922a4a546c

Comment 21

[David Bolter [:davidb] (NeedInfo me for attention)]

Based on DUPED bug 1279189 I'm assuming this affects 49. If so please request uplift for risk analysis.

Comment 22

[Ada [:adalucinet]]

Reproduced with 50.0a1 from 2016-07-06 under Windows 10 64-bit.
Verified using latest Nightly 50.0a1, across platforms [1].
Found 1 issue - bug 1287090. Since the new report is not related with the fix here, marking accordingly.

[1] Windows 10 64-bit, Mac OS X 10.9.5 and Ubuntu 16.04 64-bit