Set As Desktop Background preview uses the wrong monitor ratio for most users

RESOLVED FIXED in Firefox 68

Status

()

defect
P3
normal
RESOLVED FIXED
a month ago
a month ago

People

(Reporter: Kwan, Assigned: Kwan)

Tracking

unspecified
Firefox 68
Points:
---

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

a month ago

When the monitor_16-10.png file was introduced in bug 386163 in 2007, Wikipedia claims that 16:10 was a fairly common display ratio. However with the rise of HD, computer displays have followed TVs in mostly being 16:9, as can be observed in the hardware stats collected from Firefox users, where as of 3rd March 2019 the top three resolutions, comprising 67% of Fx users, are all 16:9 (1366x768 30.667%, 1080p 28.054%, 1600x900 8.314%).

When viewing the Desktop Background preview on a 16:9 monitor currently the preview is displayed in a canvas bounded to a 16:10 frame, but with a co-ordinate system in the monitor's 16:9. This leads to horizontal compression, as can be seeing by trying "Set As Desktop Background" on an image of a circle like this one, which ends up decidedly ellipsoidal. (Note: don't try this on release currently, only a Nightly or 67 beta with bug 1529363 fixed)

(Assignee)

Comment 1

a month ago

Currently for any screen with a ratio 1.6 or above, the preview uses a 16:10
image. However, the majority of Fx users have a screen that is 16:9[0], so for
most users the preview shows images distorted (compressed horizontally).

Originally I just added a new 16:9 version of the monitor image, but then I
realised I could save on filesize and make it responsive to whatever the
user's screen actually is, rather than using arbitrary presets, by using
border-image.

The new image files are just sliced up versions of the original monitor.png
files, zopfli compressed to match (though with the power indicator dropped from
the Linux/Windows version to avoid distorting it). The combined filesize
savings seem to be 8.5KiB on macOS and 6.5KiB on Linux/Windows.

With the removal of the use of margins on the canvas we no longer need the
platform-specific setDesktopBackground.css file.

[0] https://data.firefox.com/dashboard/hardware
As of 3rd March 2019 the top three resolutions, 1366x768, 1080p, & 1600x900,
are all 16:9 and make up 67% of the userbase.

Priority: -- → P3

Comment 3

a month ago

Pushed by ncsoregi@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e5f1bcb6046e
Use a responsive monitor frame in "Set As Desktop Background" preview. r=Gijs

Keywords: checkin-needed

Backed out changeset e5f1bcb6046e (bug 1534475) for failing at browser_setDesktopBackgroundPreview.js on a CLOSED TREE.

Backout link: https://hg.mozilla.org/integration/autoland/rev/ca8a2a294a18aa1bc5b0f764527ab6c9cd518b7d

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=a14e6fd4010b2f4e07a3353edf4971926566ce6f&selectedJob=233747226

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=233747226&repo=autoland&lineNumber=12743

Log snippet:

23:31:14 INFO - TEST-START | browser/components/shell/test/browser_setDesktopBackgroundPreview.js
23:31:14 INFO - GECKO(532) | Chrome file doesn't exist: Z:\task_1552517345\build\tests\mochitest\browser\browser\components\shell\test\head.js
23:31:14 INFO - GECKO(532) | ++DOCSHELL 13801400 == 7 [pid = 5524] [id = {471d2b9c-c094-4acd-b17d-a7931944af44}]
23:31:14 INFO - GECKO(532) | ++DOMWINDOW == 15 (188535E0) [pid = 5524] [serial = 15] [outer = 00000000]
23:31:15 INFO - GECKO(532) | ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to c:\users\task_1552517345\appdata\local\temp\tmpakyd9o.mozrunner\runtests_leaks_tab_pid5212.log
23:31:15 INFO - GECKO(532) | ++DOMWINDOW == 16 (188B7400) [pid = 5524] [serial = 16] [outer = 188535E0]
23:31:15 INFO - GECKO(532) | [Parent 5524, Main Thread] WARNING: '!ClientIsValidPrincipalInfo(mClientInfo.PrincipalInfo())', file z:/build/build/src/dom/clients/manager/ClientSource.cpp, line 169
23:31:15 INFO - GECKO(532) | [Parent 5524, Main Thread] WARNING: '!mSelection', file z:/build/build/src/editor/libeditor/EditorBase.cpp, line 4935
23:31:15 INFO - GECKO(532) | [Parent 5524, Main Thread] WARNING: '!editActionData.CanHandle()', file z:/build/build/src/editor/libeditor/EditorBase.cpp, line 1293
23:31:15 INFO - GECKO(532) | [Child 5212, Main Thread] WARNING: Failed to acquire a DXGI adapter for enumerating outputs.: file z:/build/build/src/gfx/thebes/DeviceManagerDx.cpp, line 145
23:31:15 INFO - GECKO(532) | [Parent 5524, Main Thread] WARNING: '!parent', file z:/build/build/src/netwerk/ipc/NeckoParent.cpp, line 954
23:31:15 INFO - GECKO(532) | ++DOMWINDOW == 17 (195D6000) [pid = 5524] [serial = 17] [outer = 188535E0]
23:31:15 INFO - GECKO(532) | ++DOCSHELL 01668000 == 1 [pid = 5212] [id = {1a28ebb7-01c7-4b1c-bca1-fa593d99143e}]
23:31:15 INFO - GECKO(532) | [Child 5212, Main Thread] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/widget/windows/nsLookAndFeel.cpp, line 853
23:31:15 INFO - GECKO(532) | ++DOMWINDOW == 1 (016F1280) [pid = 5212] [serial = 1] [outer = 00000000]
23:31:15 INFO - GECKO(532) | ++DOMWINDOW == 2 (0166A400) [pid = 5212] [serial = 2] [outer = 016F1280]
23:31:15 INFO - GECKO(532) | [Child 5212, Main Thread] WARNING: 'NS_FAILED(GetAccentColor(unused))', file z:/build/build/src/widget/windows/nsLookAndFeel.cpp, line 481
23:31:15 INFO - GECKO(532) | ++DOMWINDOW == 3 (089A9C00) [pid = 5212] [serial = 3] [outer = 016F1280]
23:31:15 INFO - GECKO(532) | ++DOCSHELL 15617800 == 8 [pid = 5524] [id = {bd82217c-3ec9-4b5f-96f1-48bc2df1f5be}]
23:31:15 INFO - GECKO(532) | ++DOMWINDOW == 18 (1560AEE0) [pid = 5524] [serial = 18] [outer = 00000000]
23:31:15 INFO - GECKO(532) | ++DOMWINDOW == 19 (15618800) [pid = 5524] [serial = 19] [outer = 1560AEE0]
23:31:15 INFO - GECKO(532) | [Parent 5524, Main Thread] WARNING: NS_ENSURE_TRUE(root) failed: file z:/build/build/src/layout/base/nsDocumentViewer.cpp, line 3193
23:31:15 INFO - TEST-INFO | started process screenshot
23:31:15 INFO - TEST-INFO | screenshot: exit 0
23:31:15 INFO - Buffered messages logged at 23:31:14
23:31:15 INFO - Entering test bound
23:31:15 INFO - Buffered messages logged at 23:31:15
23:31:15 INFO - Global property added while loading chrome://browser/content/nsContextMenu.js: gContextMenuContentData
23:31:15 INFO - Global property added while loading chrome://browser/content/nsContextMenu.js: SpellCheckHelper
23:31:15 INFO - Global property added while loading chrome://browser/content/nsContextMenu.js: LoginManagerContextMenu
23:31:15 INFO - Global property added while loading chrome://browser/content/nsContextMenu.js: DevToolsShim
23:31:15 INFO - TEST-PASS | browser/components/shell/test/browser_setDesktopBackgroundPreview.js | Opened correct window - "Shell:SetDesktopBackground" == "Shell:SetDesktopBackground" -
23:31:15 INFO - Buffered messages finished
23:31:15 INFO - TEST-UNEXPECTED-FAIL | browser/components/shell/test/browser_setDesktopBackgroundPreview.js | Preview's aspect ratio matches screen's - 124 == 125 - JS frame :: chrome://mochitests/content/browser/browser/components/shell/test/browser_setDesktopBackgroundPreview.js :: <TOP_LEVEL> :: line 51
23:31:15 INFO - Stack trace:
23:31:15 INFO - chrome://mochitests/content/browser/browser/components/shell/test/browser_setDesktopBackgroundPreview.js:null:51
23:31:15 INFO - resource://testing-common/BrowserTestUtils.jsm:withNewTab:111
23:31:15 INFO - chrome://mochitests/content/browser/browser/components/shell/test/browser_setDesktopBackgroundPreview.js:null:10
23:31:15 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1106
23:31:15 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1134
23:31:15 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:995
23:31:15 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803
23:31:15 INFO - GECKO(532) | [Parent 5524, Main Thread] WARNING: NS_ENSURE_TRUE(mDoneSetup) failed: file z:/build/build/src/editor/composer/nsEditingSession.cpp, line 1215
23:31:15 INFO - Leaving test bound
23:31:15 INFO - GECKO(532) | MEMORY STAT | vsize 746MB | vsizeMaxContiguous 526MB | residentFast 251MB | heapAllocated 116MB
23:31:15 INFO - TEST-OK | browser/components/shell/test/browser_setDesktopBackgroundPreview.js | took 733ms
23:31:15 INFO - checking window state

Flags: needinfo?(moz-ian)
(Assignee)

Updated

a month ago
Duplicate of this bug: 1535172
(Assignee)

Comment 6

a month ago

Green try (three known intermittents):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=66dbe6d2a0a82b67bb4a878071696c8e3a49d99a

(actually ran the whole suite this time rather than relying on test_paths, since that's apparently broken (bug 1523303))

Flags: needinfo?(moz-ian)
Keywords: checkin-needed

Hi! Please ask for a review+ for the current diff.

Flags: needinfo?(moz-ian)
Keywords: checkin-needed

Comment 8

a month ago
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/ae44c37e9e21
Use a responsive monitor frame in "Set As Desktop Background" preview. r=Gijs
(Assignee)

Updated

a month ago
Flags: needinfo?(moz-ian)

Comment 9

a month ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: a month ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
You need to log in before you can comment on or make changes to this bug.