1403202 - Near-zero contrast for Firefox name in 57 installer on Mac

Status: RESOLVED FIXED

(Firefox :: Theme, defect, P1)

Description

[Mike Taylor [:miketaylr]]

Attachment: screenshot

I downloaded the dmg installer for 57 beta and it's impossible to read the Firefox below the logo.

Comment 1

[Mike Taylor [:miketaylr]]

requesting tracking because this looks pretty terrible, and maybe there's a11y concerns.

Comment 3

[Justin Dolske [:Dolske]]

Oh, the irony that the installer visuals were totally busted for a long time without anyone filing a bug, and as soon as we fix bug 1399731 everyone reports an issue... :)

Out of curiosity, what macOS version are people seeing this running? For me the Firefox icon is always selected and has a suitable background applied. Maybe this is version-dependent?

Comment 4

[Justin Dolske [:Dolske]]

Attachment: Screenshot on DevEd

Oh, I just realized this wasn't about DevEdition. Though curious that DevEdition seems to be ok! Maybe we need a .DSStore tweak to make it selected / selectable?

Comment 5

[Mike Taylor [:miketaylr]]

(In reply to Justin Dolske [:Dolske] from comment #3)
> Oh, the irony that the installer visuals were totally busted for a long time
> without anyone filing a bug, and as soon as we fix bug 1399731 everyone
> reports an issue... :)

I see this in 10.13 High Sierra.

Comment 6

[Justin Dolske [:Dolske]]

Attachment: Screenshot on Beta ok?

Hmm, well, it's actually OK for me on Beta as well. I'm not able to so shift focus/selection such that I don't have the grey background behind the icon label.

Comment 7

[Justin Dolske [:Dolske]]

I'm on 10.12.

Comment 8

[Mike Taylor [:miketaylr]]

Attachment: Screenshot in DevEdition on High Sierra

Seems just as bad for DevEdition in High Sierra as well.

Comment 9

[Molly Howell (she/her) (mostly inactive)]

We have had a report of this from Dev Edition as well (bug 1403245).

Comment 10

[Mike Hommey [:glandium]]

Relatedly, there's bug 713811 when the file name doesn't match exactly what's in the dsstore, which tells me we should maybe try to generate the dsstore instead of creating it manually (4 slightly different versions, at that) and checking it in the repo.

Comment 11

[Mike Hommey [:glandium]]

(In reply to Justin Dolske [:Dolske] from comment #6)
> Created attachment 8912320 [details]
> Screenshot on Beta ok?
> 
> Hmm, well, it's actually OK for me on Beta as well. I'm not able to so shift
> focus/selection such that I don't have the grey background behind the icon
> label.

See that Applications logo? There's a label under it, but you can't see it. Unselect Firefox, and you won't see it either.

Comment 12

[Justin Dolske [:Dolske]]

(In reply to Mike Hommey [:glandium] from comment #11)

> > Hmm, well, it's actually OK for me on Beta as well. I'm not able to so shift
> > focus/selection such that I don't have the grey background behind the icon
> > label.
> 
> See that Applications logo? There's a label under it, but you can't see it.
> Unselect Firefox, and you won't see it either.

Yes, except as I wrote, I'm not able to actually unselect the Firefox icon, so everything looks fine. Actually, it seems this isn't actually even related to selection -- on 10.12 macOS simply appears to always give icon labels a light semitransparent background when the folder uses a background image (presumably to avoid this exact problem). 

There isn't actually a useful label under the Applications folder, because the shortcut is named " ".

If the "Firefox" label was similarly invisible, this would would a fairly minor issue. The bigger problem here is that the screenshots indicate it's _partially_ visible -- due to fringing from antialiasing -- and _that_ makes this look bad.

Not sure we've got any great options here:

* Assume this is a 10.13 bug. Do nothing and wait for Apple to fix it. (ha)

* Change the background to something lighter, so the antialiased text doesn't look awkward.

* Put a little white squircle into the background image, exactly where the label would be.


Perhaps someone with 10.13 can see if fiddling with the background color (of a normal folder) happens to also affect the label antialiasing color (with a background image)? Bit of a longshot, but maybe...

Comment 13

[Molly Howell (she/her) (mostly inactive)]

(In reply to Justin Dolske [:Dolske] from comment #12)
> Perhaps someone with 10.13 can see if fiddling with the background color (of
> a normal folder) happens to also affect the label antialiasing color (with a
> background image)? Bit of a longshot, but maybe...

Yeah, it's not gone well. Labels in normal folders look just as crappy with a background image set as our DMG does. Setting a solid color doesn't show the aliasing problem, but I don't see a way to change the text color from black, so it quickly becomes unreadable anyway.

Comment 14

[Stephen Horlander [:shorlander]]

I will look into a way to fix this with the background image, but it's a little tricky because the placement is kind of fickle.

(In reply to Mike Hommey [:glandium] from comment #10)
> Relatedly, there's bug 713811 when the file name doesn't match exactly
> what's in the dsstore, which tells me we should maybe try to generate the
> dsstore instead of creating it manually (4 slightly different versions, at
> that) and checking it in the repo.

If someone wants to figure out how to do that, that would be awesome :) I have been fiddling with these for a long time and it's always a pain.

Comment 15

[Stephen Horlander [:shorlander]]

Attachment: update-macos-dmg-backgrounds.patch

I put a background behind the label area.

Comment 16

[Justin Dolske [:Dolske]]

Comment on attachment 8912760 [details] [diff] [review]
update-macos-dmg-backgrounds.patch

(also looks fine on pre-10.13)

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/69c3cb2e731c
Update macOS DMG Backgrounds for label reability. r=dolske

Comment 18

[Justin Dolske [:Dolske]]

Comment on attachment 8912760 [details] [diff] [review]
update-macos-dmg-backgrounds.patch

Approval Request Comment
[Feature/Bug causing the regression]: n/a - macOS regression
[User impact if declined]: ugly text label in DMG
[Is this code covered by automated tests?]: n/a
[Has the fix been verified in Nightly?]: no, manually verified in local build
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: No
[Why is the change risky/not risky?]: Simple image swap.
[String changes made/needed]: n/a

Comment 19

[Sylvestre Ledru [:Sylvestre]]

Tracking as we clearly want to land that.

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/69c3cb2e731c

Comment 21

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8912760 [details] [diff] [review]
update-macos-dmg-backgrounds.patch

Visual regression, taking it.
Should be in 57b5

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/abbc7bcb54e3

Comment 23

[Andrei Vaida [:avaida]]

(In reply to Justin Dolske [:Dolske] from comment #18)
> [Is this code covered by automated tests?]: n/a
> [Has the fix been verified in Nightly?]: no, manually verified in local build
> [Needs manual test from QE? If yes, steps to reproduce]: no

Setting qe-verify- based on Justin's assessment on manual testing needs.