Closed Bug 1557218 Opened 5 years ago Closed 5 years ago

[10.15] system icons on macos 10.15 don't work

Categories

(Core :: Graphics: ImageLib, defect, P2)

Unspecified
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox-esr60 68+ fixed
firefox-esr68 --- fixed
firefox67 --- wontfix
firefox68 + fixed
firefox69 + fixed

People

(Reporter: tnikkel, Assigned: mstange)

References

(Blocks 1 open bug)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file)

See bug 1556076 comment 25. We fail that line for some reason, so we probably can't draw some (any?) icons we get from the system. I haven't verified this because I don't have 10.15 but I can't see how it would work.

OS: Unspecified → macOS
Priority: -- → P3
Whiteboard: [gfx-noted]
Priority: P3 → P2

From Apple:

Firefox shouldn’t be making assumptions about the bitmap format produced by -initWithFocusedViewRect. They shouldn’t be using -initWithFocusedViewRect: anyway — if they want bitmap data of an image, they should create their own bitmap context, whose format they’d control, and draw the image into it.

I'll create a patch that does what they recommend.

Assignee: nobody → mstange
Status: NEW → ASSIGNED
Pushed by mstange@themasta.com: https://hg.mozilla.org/integration/autoland/rev/55d83c717bc8 Get the bitmap data for system icons by drawing them to a CGBitmapContext with a format of our choosing. r=tnikkel

I've only tested this fix on 10.12; I'm pretty confident it will fix the problem on 10.15 but it still needs to be verified.

We're going to want this on 68 for sure. IIUC, ESR60 isn't affected, however. Please feel free to correct me if that's wrong, however.

We should probably get a Beta uplift request on this ASAP so we can uplift this for 68b14 tomorrow if possible, assuming it sticks in the mean time :-).

Flags: needinfo?(mstange)

(In reply to Ryan VanderMeulen [:RyanVM] from comment #5)

IIUC, ESR60 isn't affected, however. Please feel free to correct me if that's wrong, however.

All versions are affected in the sense that they won't draw these icons on 10.15. (This code was 13 years old.)
67 and 67.0.1 (which have the patch from bug 1520868 but not one from bug 1556076) were additionally affected in the sense that they were crashing.

I recommend uplifting this patch to ESR60, too.

Flags: needinfo?(mstange)

Thanks for clarifying!

Flags: qe-verify+

Comment on attachment 9074368 [details]
Bug 1557218 - Get the bitmap data for system icons by drawing them to a CGBitmapContext with a format of our choosing. r?tnikkel

Beta/Release Uplift Approval Request

  • User impact if declined: Missing icons in some places in the UI when running on macOS 10.15.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Be on a macOS 10.15 Beta. A: Open preferences. Scroll down to the Applications section. Make sure the table shows the DMG and PDF file type icons in the respective rows. B: Download a variety of files of different types. Open the downloads panel or the downloads page of the Library window. Make sure all entries have appropriate file type icons.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Small fix, and the code is just not very complicated.
  • String changes made/needed: none

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: macOS update is exposing a bug in Firefox
  • User impact if declined: Missing icons in some places in the UI when running on macOS 10.15.
  • Fix Landed on Version: 69
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Small fix, and the code is just not very complicated.
  • String or UUID changes made by this patch: none
Attachment #9074368 - Flags: approval-mozilla-esr68?
Attachment #9074368 - Flags: approval-mozilla-esr60?
Attachment #9074368 - Flags: approval-mozilla-beta?
Backout by malexandru@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5ffc2eb8ac9f Backed out changeset 55d83c717bc8 for causing reftest failures. CLOSED TREE

The reftest failure was caused by the one behavior change in the patch: we now always double the size for HiDPI, instead of only doing it whenever -[NSBitmapRep initWithFocusedViewRect:] chose to apply HiDPI resolution. (Which I think it does if there's at least one HiDPI monitor connected? Not sure.) And in automation the reftest is run on a machine without a HiDPI monitor, so the size was not doubled in the past.
However, when I run the reftest on my machine, which does have a HiDPI monitor, the same reftest already fails!
So I'm going to change the test, so that it deals with images which are larger than the requested size correctly.

Flags: needinfo?(mstange)
Pushed by mstange@themasta.com: https://hg.mozilla.org/integration/autoland/rev/98a36d0a7ec8 Get the bitmap data for system icons by drawing them to a CGBitmapContext with a format of our choosing. r=tnikkel
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

Hi Brindusa, can your team please prioritize Nightly verification of this bug? Thanks in advance!

Flags: needinfo?(brindusa.tot)

I tried to reproduce this bug in order to verify the fix but I couldn't reproduce it on Mac OS X 10.15 but not on HiDPI resolution because we don't have any Mac OS X 10.15 with HiDPI monitor.
File type icons looks the same as on other macOS versions in "preferences" page and in the downloads panel.

Is there any other way we could verify this bug?
Thanks.

Flags: needinfo?(brindusa.tot) → needinfo?(mstange)

(In reply to Hani Yacoub from comment #16)

I tried to reproduce this bug in order to verify the fix but I couldn't reproduce it on Mac OS X 10.15 but not on HiDPI resolution because we don't have any Mac OS X 10.15 with HiDPI monitor.

Don't worry about HiDPI or not HiDPI. If you can verify the fix on a non-HiDPI monitor, that's enough.
Which version of the 10.15 Beta are you using? It's possible that this bug only occurs on certain versions of the Beta; the crash reports for a related bug happened on the versions 19A471t, 19A487l and 19A487m. It is possible that Apple has shipped a more recent Beta which works around the bug on their side.

(I don't have 10.15 myself unfortunately.)

Flags: needinfo?(mstange)

I can verify using the latest 10.15 version (19A487l) on nightly using the steps in Comment 9. What still isn't working for me is Bug 1561599 which is duped to this bug.

Thanks, Marcia! I've unduped the other bug.

Comment on attachment 9074368 [details]
Bug 1557218 - Get the bitmap data for system icons by drawing them to a CGBitmapContext with a format of our choosing. r?tnikkel

fix for macos 10.15, verified in 69, approved for 68 rc1

Attachment #9074368 - Flags: approval-mozilla-release+
Attachment #9074368 - Flags: approval-mozilla-esr68?
Attachment #9074368 - Flags: approval-mozilla-esr68+
Attachment #9074368 - Flags: approval-mozilla-beta?

Comment on attachment 9074368 [details]
Bug 1557218 - Get the bitmap data for system icons by drawing them to a CGBitmapContext with a format of our choosing. r?tnikkel

Approved for 60.8esr also.

Attachment #9074368 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+

I am using 10.15 Beta (19A487m), I tried on a Firefox Nightly 69.0a1 (2019-06-05) and on Firefox Nightly 69.0a1 (2019-06-26) and I couldn't reproduce the issue.
File type icons looks the same as on other macOS versions in "preferences" page and in the downloads panel.

QA Whiteboard: [qa-triaged]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: