Closed Bug 1454610 Opened 2 years ago Closed Last year

DMG quantum background image not properly sized

Categories

(Firefox :: Theme, defect, P2)

All
macOS
defect

Tracking

()

VERIFIED FIXED
Firefox 63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 62+ verified
firefox59 --- unaffected
firefox60 - wontfix
firefox61 - verified
firefox62 + verified
firefox63 --- verified

People

(Reporter: Pike, Assigned: timdream)

References

Details

(Keywords: regression)

Attachments

(2 files)

While trying the DMG from http://archive.mozilla.org/pub/firefox/candidates/60.0b8-candidates/build1/mac/de/, I found that the background graphic gets the scaling wrong.

Attaching a screenshot.

I have a non-retina secondary monitor, but the dialog showed on my retina display, maybe that's part of it.
Component: Untriaged → Installer
Reproducible on Mac OS, all Quantum builds.
OS: Unspecified → Mac OS X
Hardware: Unspecified → All
Yeah, this reproduces on my non-retina MacBook too (running 10.13.4). Between this and bug 1439072 I'm definitely starting to think we're coming up against Finder bugs.
Component: Installer → Theme
:spohl, do you have any ideas about this, or know someone better to ask? Because I've got nothing.
Flags: needinfo?(spohl.mozilla.bugs)
I don't know off-hand. I would set a need-info on whoever added this background graphic to see if this is a known issue and to get this addressed.
Flags: needinfo?(spohl.mozilla.bugs)
Looking into that is taking me through a whole list of people who are no longer active. :( Well, and also :glandium, but this doesn't really seem like his problem. I wouldn't mind owning this thing myself, I just don't know how to even start investigating these bugs; my Mac expertise is limited anyway, and this is a particularly esoteric corner even within that realm. But I'll see if there's anything I can do.

Also, I've just discovered in the process of looking into this bug that the current dev edition release just doesn't show the background art at all, apparently because the volume name isn't what the prebaked .DS_Store file expects. So that's cool. I'll file that separately, I guess.
Feel free to kick this over to widget:cocoa and mark it as P2 if you can't find the time to look into it. I will prioritize alongside other platform work.
Okay, you've got bug 1455768. Congratulations. ;)

I'll move this and bug 1439072 over there also.
Component: Theme → Widget: Cocoa
Product: Firefox → Core
Priority: -- → P2
Duplicate of this bug: 1466068
This affects Firefox from 60.0, including 60.0esr and the 61.0 betas. Not present in 59.0.
This has broken before, the process to make this all work is a bit delicate/complicated...

See bug 1435359 comment 2 on the steps that are normally needed to do this.

I don't see any changes to the DSStore file since, although bug 1234008 did touch the background.png used... Stephen, was there anything special about the PNG used that might have been affected by the image optimizer?

I wonder if we need to figure out a way to automatically test this... Screenshot the Finder window, and compare with a reference image? Add it to the QA manual verification for Release?
Flags: needinfo?(shorlander)
Tim: Anything you see in the old-vs-new background.png that changed?
Flags: needinfo?(timdream)
Comment 1 and comment 9 contracts to one another.

Anyway, I've tried to see https://ftp.mozilla.org/pub/firefox/releases/59.0.3/mac/en-US/Firefox%2059.0.3.dmg and it shows up correctly. Nightly seems unaffected.

Will dig deeper... keeping needinfo.
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #12)
> Nightly seems unaffected.

Nightly is not affected because the image is not hi-res anyway.

From tools, I can see the original background.png comes with DPI set to 144 pixels/inch.
https://hg.mozilla.org/mozilla-central/raw-file/69c3cb2e731c/browser/branding/official/background.png

Playing around with Preview.app and exiftools I can see the following command will allow the new file to be displayed in the same dimensions as the original file in Preview.app.

exiftool -XResolution=144 -YResolution=144 -PixelsPerUnitX=5669 -PixelsPerUnitY=5669 background.png

I'll push a try build with this change.
Assignee: nobody → timdream
Status: NEW → ASSIGNED
Flags: needinfo?(timdream)
Attachment #8984884 - Flags: review?(dolske)
Try result confirms this bug can be fixed by comment 13.

I am not entirely sure how to prevent this footgun in the future. Taking a screenshot of the Finder window is perhaps an overkill?

How about implementing a small PNG parser in mochitest to read out the metadata tags? Would that be sufficient? Do we have that in the tree already?
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #16)
> How about implementing a small PNG parser in mochitest to read out the
> metadata tags? Would that be sufficient? Do we have that in the tree already?

Hum, that won't work -- we don't put these files in the jarfiles.
This affects install experiences, perhaps we should uplift.
Too late for ESR 60.1 and Fx61 at this point (we're already building RCs), but this seems like a good dot release ride-along candidate should the opportunity arise.
(In reply to Justin Dolske [:Dolske] from comment #10)
> This has broken before, the process to make this all work is a bit
> delicate/complicated...
> 
> See bug 1435359 comment 2 on the steps that are normally needed to do this.
> 
> I don't see any changes to the DSStore file since, although bug 1234008 did
> touch the background.png used... Stephen, was there anything special about
> the PNG used that might have been affected by the image optimizer?
> 
> I wonder if we need to figure out a way to automatically test this...
> Screenshot the Finder window, and compare with a reference image? Add it to
> the QA manual verification for Release?

Reading the rest of the comments, it seems like the issue is some processing of PNGs stripping the meta information from the PNG?
Flags: needinfo?(shorlander)
Duplicate of this bug: 1470456
Component: Widget: Cocoa → Theme
Product: Core → Firefox
Comment on attachment 8984884 [details]
Bug 1454610 - Restore PNG density metadata on DMG backgrounds

https://reviewboard.mozilla.org/r/250662/#review259846
Comment on attachment 8984884 [details]
Bug 1454610 - Restore PNG density metadata on DMG backgrounds

https://reviewboard.mozilla.org/r/250662/#review259848
Attachment #8984884 - Flags: review?(dolske) → review+
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #16)

> I am not entirely sure how to prevent this footgun in the future. Taking a
> screenshot of the Finder window is perhaps an overkill?
> 
> How about implementing a small PNG parser in mochitest to read out the
> metadata tags? Would that be sufficient? Do we have that in the tree already?

Yeah, I'm not really sure we have test infra set up for that. You'd also need the test to be able to differentiate between Nightly/DevEd/Release, and screenshots can be annoying if slight differences exist (e.g. maybe scaling is slightly different on some OS versions).

Maybe easiest thing to do here is have QA add this to their list of periodic release checks?
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #13)

> Nightly is not affected because the image is not hi-res anyway.

Do we want a hidpi image for the Nightly DMG? If so, new bug w/ desired image.
Flags: needinfo?(shorlander)
(In reply to Justin Dolske [:Dolske] from comment #25)
> Do we want a hidpi image for the Nightly DMG? If so, new bug w/ desired
> image.

Yes, let me know if there is an asset ready and I can compress and land it.
Pushed by timdream@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/19a63eb13bc2
Restore PNG density metadata on DMG backgrounds r=dolske
https://hg.mozilla.org/mozilla-central/rev/19a63eb13bc2
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Comment on attachment 8984884 [details]
Bug 1454610 - Restore PNG density metadata on DMG backgrounds

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1234008
[User impact if declined]: Mac installer will come with an incorrect background
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Cannot be verified since this only affect official branding. Verified on Try in comment 15.
[Needs manual test from QE? If yes, steps to reproduce]: Manually inspect the Mac installer on Beta and official build
[List of other uplifts needed for the feature/fix]: No.
[Is the change risky?]: No
[Why is the change risky/not risky?]: Just an asset update.
[String changes made/needed]: None.

Can this also ride-along a dot-release in 61? Do we want to take this in esr60 too?
Attachment #8984884 - Flags: approval-mozilla-release?
Attachment #8984884 - Flags: approval-mozilla-beta?
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #26)
> (In reply to Justin Dolske [:Dolske] from comment #25)
> > Do we want a hidpi image for the Nightly DMG? If so, new bug w/ desired
> > image.
> 
> Yes, let me know if there is an asset ready and I can compress and land it.

Created bug 1471657
Flags: needinfo?(shorlander)
Comment on attachment 8984884 [details]
Bug 1454610 - Restore PNG density metadata on DMG backgrounds

Fix for recent regression. Let's take this for beta 4 and verify it in beta.
Attachment #8984884 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Managed to reproduce  with the link provided in comment 0. 
Fixed on latest nightly (63.0a1 - 2018-06-29).

Still reproducible on 62.0b4, Timothy can you take a look at this please?
Flags: needinfo?(timdream)
I cannot reproduce this on 62.0b4, downloaded from this URL

https://download-installer.cdn.mozilla.net/pub/firefox/releases/62.0b4/mac/en-US/Firefox%2062.0b4.dmg

(redirected from https://download.mozilla.org/?product=firefox-beta-latest&os=osx )

I am on macOS 10.12.6.

Perhaps this can only reproduce on a different macOS version?
Flags: needinfo?(timdream) → needinfo?(cristian.fogel)
Rechecked on macOS: 10.9.5, 10.11.6, 10.13.4.

The fix is working after all with both the provided link and a download from http://archive.mozilla.org/pub/firefox/releases/62.0b4/mac/en-US/

During the re-testing I've noticed a scenario that was causing the issue to re-appear:
- opening the installer from the affected build;
- close it and open the 62.0b4 build.
The layout is still the broken one.

How should we proceed next; mark this as fixed and open up a follow-up bug or [...]?
Flags: needinfo?(cristian.fogel) → needinfo?(timdream)
Comment on attachment 8984884 [details]
Bug 1454610 - Restore PNG density metadata on DMG backgrounds

Taking this as a ride-along for Fx 61.0.1. Please also request ESR60 approval on it when you get a chance.
Attachment #8984884 - Flags: approval-mozilla-release? → approval-mozilla-release+
Attachment #8984884 - Flags: approval-mozilla-esr60?
(In reply to Cristi Fogel [:cfogel] from comment #35)
> Rechecked on macOS: 10.9.5, 10.11.6, 10.13.4.
> 
> The fix is working after all with both the provided link and a download from
> http://archive.mozilla.org/pub/firefox/releases/62.0b4/mac/en-US/
> 
> During the re-testing I've noticed a scenario that was causing the issue to
> re-appear:
> - opening the installer from the affected build;
> - close it and open the 62.0b4 build.
> The layout is still the broken one.
> 
> How should we proceed next; mark this as fixed and open up a follow-up bug
> or [...]?

Please file a new bug. Mounting different builds is unlikely something ordinary users do every day. Thanks.
Flags: needinfo?(timdream)
(In reply to Ryan VanderMeulen [:RyanVM][PTO July 4-9] from comment #36)
> Taking this as a ride-along for Fx 61.0.1. Please also request ESR60
> approval on it when you get a chance.

Given that you have the flag that already I assume I don't need to do anything more here?
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #38) 
> Please file a new bug. Mounting different builds is unlikely something
> ordinary users do every day. Thanks.

New bug can be found at: https://bugzilla.mozilla.org/show_bug.cgi?id=1472937
Marking firefox62 as verified.
Fix confirmed on 61.0.1 as well.
Flags: qe-verify+
Updating bug status as well.
Status: RESOLVED → VERIFIED
Comment on attachment 8984884 [details]
Bug 1454610 - Restore PNG density metadata on DMG backgrounds

Approved for ESR 60.2 also.
Attachment #8984884 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Flags: qe-verify+
Fix confirmed on ESR60, using the build(s) from taskCluster.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.