1572840 - RDM pane should scale up/down with changes to full-page zoom

Status: RESOLVED FIXED

(DevTools :: Responsive Design Mode, defect, P1)

Description

[Brad Werth [:bradwerth]]

When changing full-page zoom with RDM enabled, the RDM pane itself stays the same size. This has the undesirable effect of displaying content on the simulated device screen that could not actually be reached on a real device. It would be better for us to scale up/down the RDM pane itself along with the contents, simulating instead the experience of the device getting bigger/smaller, or perhaps closer to/farther from the viewer.

When changing the size of the RDM pane in this way, the reported pixel resolution of the device should not change.

This is also what Chrome developer tools does and may be better aligned with our users' expectations.

Comment 1

[Brad Werth [:bradwerth]]

Attachment: Bug 1572840 Part 1: Add a zoom viewport action to RDM pane.

Comment 2

[Brad Werth [:bradwerth]]

Attachment: Bug 1572840 Part 2: Make the RDM viewport render scaled to the viewport.zoom property.

Depends on D41463

Comment 3

[Brad Werth [:bradwerth]]

Attachment: Bug 1572840 Part 3: Listen to fullzoom changes in the RDM pane and call the zoom viewport function.

Depends on D41465

Comment 4

[Brad Werth [:bradwerth]]

Okay, the updated patches now kind of work. While we discuss whether or not we want to make this UI change, I will put the code up for review to address anything technically wrong with it.

Comment 5

[Brad Werth [:bradwerth]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=661685206db286901c6ca9a1d04ff42ca4d6a1cd

Comment 6

[Brad Werth [:bradwerth]]

Attachment: Bug 1572840 Part 5: Update test expectations.

Depends on D41467

Comment 7

[Brad Werth [:bradwerth]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5bd293e39c26658993f90c30054122f09b6a11f0

Comment 8

[Brad Werth [:bradwerth]]

Attachment: Bug 1572840 Part 4: Change nsGlobalWindowOuter::GetRDMDeviceSize to divide out full zoom levels.

Depends on D41467

Comment 9

[Brad Werth [:bradwerth]]

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

Comment 10

[Brad Werth [:bradwerth]]

Attachment: Bug 1572840 Part 5: Update test expectations.

https://phabricator.services.mozilla.com/D42379

Depends on D43716

Comment 11

[Brad Werth [:bradwerth]]

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

Comment 12

[Brad Werth [:bradwerth]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0db07ad74a3e3f9b2f5f5a502b1b7c98e1be88c8

Comment 13

[Brad Werth [:bradwerth]]

Rounding errors when reconstituting device size in nsGlobalWindowOuter::GetRDMDeviceSize is making this challenging. With the changes in this patch, it's possible with < 100% zoom for the RDM pane to be locked to device pixels at the scaled-down size and that introduces inaccuracy when dividing out the zoom level again. This is the source of the test failures in devtools/client/responsive/test/browser/browser_window_sizing.js.

Ideally nsGlobalWindowOuter::GetRDMDeviceSize could get the size without relying on a lossy multiply by < 1, clamp to integer pixels then divide by < 1. We want screen.width and window.outerWidth to report the proper size at all zoom levels. So I don't want to just loosen up the test to accomodate a larger error value.

Comment 14

[Brad Werth [:bradwerth]]

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

Comment 15

[Brad Werth [:bradwerth]]

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

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/23feecbe07eb
Part 1: Add a zoom viewport action to RDM pane. r=gl
https://hg.mozilla.org/integration/autoland/rev/038722cc5bf4
Part 2: Make the RDM viewport render scaled to the viewport.zoom property. r=gl
https://hg.mozilla.org/integration/autoland/rev/8cd803e497e7
Part 3: Listen to fullzoom changes in the RDM pane and call the zoom viewport function. r=gl
https://hg.mozilla.org/integration/autoland/rev/db42ac54835f
Part 4: Change nsGlobalWindowOuter::GetRDMDeviceSize to divide out full zoom levels. r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/466e8f97c54f
Part 5: Update test expectations. r=gl

Comment 17

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/23feecbe07eb
https://hg.mozilla.org/mozilla-central/rev/038722cc5bf4
https://hg.mozilla.org/mozilla-central/rev/8cd803e497e7
https://hg.mozilla.org/mozilla-central/rev/db42ac54835f
https://hg.mozilla.org/mozilla-central/rev/466e8f97c54f

Comment 18

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/releases/mozilla-beta/rev/23feecbe07eb
https://hg.mozilla.org/releases/mozilla-beta/rev/038722cc5bf4
https://hg.mozilla.org/releases/mozilla-beta/rev/8cd803e497e7
https://hg.mozilla.org/releases/mozilla-beta/rev/db42ac54835f
https://hg.mozilla.org/releases/mozilla-beta/rev/466e8f97c54f