1460795 - Tooltip of devtool position is wrong when the devtool panel is zoomed in/out.

Status: VERIFIED FIXED

(DevTools :: Framework, defect, P2)

Description

[Mantaroh Yoshinaga[:mantaroh]]

This is follow up a bug of bug 14455462.

Bug 14455462 will fix the popup menu position when zooming in/out. However, the tooltip of devtool has the same problem. As julian mentioned, we will need to fix the following tooltip code:

https://searchfox.org/mozilla-central/rev/5ff2d7683078c96e4b11b8a13674daded935aa44/devtools/client/shared/widgets/tooltip/HTMLTooltip.js#601

Example:
let zoom = parseFloat(Services.prefs.getCharPref("devtools.toolbox.zoomValue"));
if (!zoom || isNaN(zoom)) {
  zoom = 1.0;
}
this.xulPanelWrapper.openPopupAtScreen(left * zoom, top, false);


STR:
 1) Open inspector
 2) Zoom in panel by using CTRL + [+].
 3) Show tooltip (e.g. show image)

AR: 
 A tooltip is displayed in the wrong position.

Comment 1

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: tooltip.png

This is a popup tooltip of an image. If I zoomed in devtool, this popup is displayed in the wrong position.

Comment 2

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: tooltip.png

The previous image didn't contain tooltip. A correct image is this attachment.

Comment 3

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: Bug 1460795 - HTML tooltips should consider the content zoom value.

This patch will multiply the HTML tooltip position to the zoom value in order to
display HTML tooltip in correct position.

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

Comment 4

[Julian Descottes [:jdescottes]]

Comment on attachment 8975690 [details]
Bug 1460795 - HTML tooltips should consider the content zoom value.

https://reviewboard.mozilla.org/r/243910/#review250538

Thanks Mantaroh, this fixes the issue and having a new test is great! 
One small comment and a suggestion, no need for another review here.

::: devtools/client/shared/test/browser_html_tooltip_zoom.js:28
(Diff revision 1)
> +  registerCleanupFunction(async function() {
> +    Services.prefs.clearUserPref("devtools.toolbox.zoomValue");
> +  });
> +  Services.prefs.setCharPref("devtools.toolbox.zoomValue", zoom.toString(10));

In tests we can use the pushPref helper:

await pushPref("devtools.toolbox.zoomValue", zoom.toString(10));

it will automatically take care of cleaning up the modified preference at the end of the test.

::: devtools/client/shared/test/browser_html_tooltip_zoom.js:49
(Diff revision 1)
> +      doc.querySelector(".tooltip-xul-wrapper").getBoxQuads({retliveTo: doc})[0].bounds;
> +  let anchorRect = doc.getElementById("box1").getBoxQuads({retliveTo: doc})[0].bounds;

retliveTo -> relativeTo

In this case, everything is in the same document so you could even skip the option.

Comment 5

[Mantaroh Yoshinaga[:mantaroh]]

Comment on attachment 8975690 [details]
Bug 1460795 - HTML tooltips should consider the content zoom value.

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

Comment 6

[Mantaroh Yoshinaga[:mantaroh]]

Comment on attachment 8975690 [details]
Bug 1460795 - HTML tooltips should consider the content zoom value.

https://reviewboard.mozilla.org/r/243910/#review250538

Thank you for the review!

> In tests we can use the pushPref helper:
> 
> await pushPref("devtools.toolbox.zoomValue", zoom.toString(10));
> 
> it will automatically take care of cleaning up the modified preference at the end of the test.

OK. I addressed this.

Comment 7

[Mantaroh Yoshinaga[:mantaroh]]

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=59d9b18eab3ca0bf5ccf8fe356662939ffe3a50c

Comment 9

[Mantaroh Yoshinaga[:mantaroh]]

(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #7)
> Try:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=59d9b18eab3ca0bf5ccf8fe356662939ffe3a50c

This test failure is bug 1454123.

I tried latest m-c:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=99cb4909d6aedae6ba23c7c09c1fad5bb3767cd0

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/99a063f6a3f2
HTML tooltips should consider the content zoom value. r=jdescottes

Comment 11

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/99a063f6a3f2

Comment 13

[Timea Cernea [:tbabos][inactive]]

Verified - Fixed on latest Nightly Nightly 66.0a1 (2018-12-18), Beta 65.0b4 and Release 64.0.
The tooltip is displayed correctly when the devtools panel is zoomed in/out.

Comment 14

[Timea Cernea [:tbabos][inactive]]

Further testing revealed a slightly different scenario that greatly affects the tooltip position and even its size. Please check the attached files:

STR:
1. Zoom in an out while the tooltip is displayed.

Actual: 
The tooltip itself will increase in size and will be misplaced.

Windows/Ubuntu behavior: https://streamable.com/3wl06
Mac 10.14: https://streamable.com/r5cmn <- major.

Patrick, should I submit a new bug for this?

Comment 15

[Patrick Brosset <:pbro>]

Yes, please file a new bug for this.
I'm tempted to say that this new bug should be a P3/minor, because this only affect users who would zoom *while* having a tooltip visible. I don't think this is likely to affect too great a number of people because tooltips are action-based. You open one to get something done, and then you close it. There's probably not a lot of chances you will zoom while it is displayed. But I could be wrong.
The other reason I think this is minor is that it is recoverable: closing and reopening the tooltip will get rid of the issue.
Finally (at least for me on macOS), it only happens when zooming out, not when zooming in (making things bigger) which, I assume, is what people use the browser zoom function for.

Comment 16

[Timea Cernea [:tbabos][inactive]]

My bad Patrick, I didn't really intend to mark it as a major issue. Just didn't know at the time being how to describe that on Mac it has a different, more visually unpleasant behavior. :(
Will submit a new bug for it and mention it here as well. Thanks for the answer and sorry for the misunderstanding.