1328011 - New broken set of APIs doesn't account zoom level, therefore tooltips, splitters and context menus appear in the wrong places

Status: RESOLVED WORKSFORME

(DevTools :: Framework, defect, P2)

Description

[arni2033]

>>>   My Info:   Win7_64, Nightly 52, 32bit, ID 20161001030430 (2016-10-01)
STR_1:
0. Open url   data:text/html,<img src="about:logo">
1. Open devtools (F12)
2. Increase zoom level in devtools (Ctrl+"=")
3. Click on iframes dropmarker

AR:  List of iframes appears detached from iframes button
ER:  List of iframes should appear near the button

This is regression from bug 1266419. Regression range:
> https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=3a87296fe4145138c2ce15512bb31f76fe869cb4&tochange=42fab251fe111d5f891c9bde0ee1fb6f7f946a50

"steps-wanted" keyword means that you should write 2 more scenarios mentioned in the summary
"regressionwindow-wanted" keyword means that you should find regression windows for all scenarios

Comment 1

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

Seems worth fixing to me...  Honza, can you evaluate further?

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

The list of frames (drop down menu) coordinates are calculated here:
https://dxr.mozilla.org/mozilla-central/rev/b1c31c4a0a678194931779e0f13fba7b508eb109/devtools/client/framework/toolbox.js#1858

The logic seems to be wrong since it doesn't respect the current zoom level. I think that fixing this should fix the reported problem.

@Sébastien: Perhaps we could use win.devicePixelRatio to fix this?

Honza

Comment 3

[Sébastien Blin [:sblin] [:amarok]]

Attachment: bug_1328011.patch

> @Sébastien: Perhaps we could use win.devicePixelRatio to fix this?

Yeap, I think we can (like I do in the attachment). But we need to check on a High DPI monitor (to avoid things like bug 1328895). If it doesn't work on a High DPI monitor, we should replace screenX, screenY and rect by value in physical screen pixels not in CSS pixels.

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

@ntim: can you please checkout the patch?

Thanks!
Honza

Comment 5

[Tim Nguyen :ntim]

Attachment: Screenshot

Seems like the top position calculation is wrong.

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8826740 [details] [diff] [review]
bug_1328011.patch

Please see comment #5

@Sébastien, any progress on this?

Honza

Comment 7

[Liz Henry (:lizzard) (relman/hg->git project)]

Too late for a fix for 53, fix-optional for 54, minor carryover regression.

Comment 8

[Patrick Brosset <:pbro>]

Tested again on macOS with Firefox 65 (nightly) and this does not happen anymore. So I'm going to close this for now.