Closed Bug 1572840 Opened 5 years ago Closed 5 years ago

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

Categories

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

Product:
DevTools
Component:
Responsive Design Mode
Type:
defect

Tracking

(firefox70 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 70
Tracking Flags:
Tracking Status
firefox70 --- fixed

People

(Reporter: bradwerth, Assigned: bradwerth)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [rdm-mvp])

Attachments

(5 files, 1 obsolete file)

Bug 1572840 Part 1: Add a zoom viewport action to RDM pane.
(deleted), text/x-phabricator-request
Details
Bug 1572840 Part 2: Make the RDM viewport render scaled to the viewport.zoom property.
(deleted), text/x-phabricator-request
Details
Bug 1572840 Part 3: Listen to fullzoom changes in the RDM pane and call the zoom viewport function.
(deleted), text/x-phabricator-request
Details
Bug 1572840 Part 4: Change nsGlobalWindowOuter::GetRDMDeviceSize to divide out full zoom levels.
(deleted), text/x-phabricator-request
Details
Bug 1572840 Part 5: Update test expectations.
(deleted), text/x-phabricator-request
Details

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.

Status: NEW → ASSIGNED
Priority: -- → P1

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.

Attached file Bug 1572840 Part 5: Update test expectations. (obsolete) (deleted) —

Depends on D41467

Depends on: 1575097
Attachment #9086179 - Attachment description: Bug 1572840 Part 4: Update test expectations. → Bug 1572840 Part 5: Update test expectations.
Attachment #9086179 - Attachment is obsolete: true

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.

Blocks: 1577775
Pushed by bwerth@mozilla.com: 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
Blocks: 1579986
Regressions: 1602799
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: