Last Comment Bug 747601 - Only create .plist file after icon has been downloaded
: Only create .plist file after icon has been downloaded
Status: VERIFIED FIXED
[marketplace-beta=]
:
Product: Firefox Graveyard
Classification: Graveyard
Component: Web Apps (show other bugs)
: Trunk
: All Mac OS X
: -- normal
: Firefox 14
Assigned To: :Felipe Gomes (needinfo me!)
: Jason Smith [:jsmith]
Mentors:
: 747414 (view as bug list)
Depends on:
Blocks: 732612 745999
  Show dependency treegraph
 
Reported: 2012-04-20 21:26 PDT by :Felipe Gomes (needinfo me!)
Modified: 2016-02-04 15:00 PST (History)
4 users (show)
jsmith: in‑moztrap+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch (1.90 KB, patch)
2012-04-21 18:45 PDT, :Felipe Gomes (needinfo me!)
mstange: review+
Details | Diff | Splinter Review

Description :Felipe Gomes (needinfo me!) 2012-04-20 21:26:15 PDT
When the .plist file is created, Finder caches the information about the app, and depending on the timing, if the .icns icon file has not been created yet, the app will get the generic app icon instead of the proper one, and Finder won't refresh the info (I think if you kill it or restart the computer it might work).

To avoid this we need to create the .plist file only after the icon has been downloaded and converted to the right format.

(There's 1 or 2 bugs already filed about icons not always working correctly on Mac which are likely duplicates of this.)
Comment 1 :Felipe Gomes (needinfo me!) 2012-04-21 18:45:00 PDT
Created attachment 617269 [details] [diff] [review]
Patch

Dan, could you reliably reproduce this problem? Can you test to see if this patch fixes it? It should implement the simpler solution that we discussed.
Comment 2 :Felipe Gomes (needinfo me!) 2012-04-22 01:50:41 PDT
Comment on attachment 617269 [details] [diff] [review]
Patch

Even if this patch doesn't fix the problem entirely (I believe it will, but I cannot reproduce the problem), it's a correct patch anyway that will be necessary, so I'm gonna go ahead and request review.

Markus, I just took the plist creation out of _createConfigFiles and into its own function. getIconForApp already had a callback parameter for when it finishes processing the icon, but it was unused. I also added some code to make sure it gets called even if downloading the icon fails.
Comment 3 :Felipe Gomes (needinfo me!) 2012-04-22 14:34:54 PDT
Leaving open to see if there's anything more that will be needed to fix the original bug.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3dd842015565
Comment 4 Ed Morley [:emorley] 2012-04-22 23:42:58 PDT
https://hg.mozilla.org/mozilla-central/rev/3dd842015565
Comment 5 Dan Walkowski 2012-04-23 16:05:46 PDT
*** Bug 747414 has been marked as a duplicate of this bug. ***
Comment 6 :Felipe Gomes (needinfo me!) 2012-04-26 07:50:28 PDT
The rest of the necessary work to fix the original bug (osx caching the incorrect icon) was covered by bug 748199
Comment 7 :Felipe Gomes (needinfo me!) 2012-04-26 08:08:53 PDT
For Jason: the two dependents bugs here are most likely fixed by this change + the change that just landed from bug 748199.
Comment 8 Jason Smith [:jsmith] 2012-04-26 15:58:42 PDT
This works on OS X 10.7. Does not work at all on OS X 10.5. Requesting to reopen.
Comment 9 Jason Smith [:jsmith] 2012-04-26 16:12:04 PDT
On OS X 10.6 - Does not fully work either. If I have the applications folder open, and I install an application, the icon may not appear. If close finder, reopen the applications directory, then I will see the icon.
Comment 10 :Felipe Gomes (needinfo me!) 2012-04-26 20:45:25 PDT
(In reply to Jason Smith [:jsmith] from comment #9)
> On OS X 10.6 - Does not fully work either. If I have the applications folder
> open, and I install an application, the icon may not appear. If close
> finder, reopen the applications directory, then I will see the icon.

Was this with a hourly mozilla-central build from this afternoon?
Comment 11 Jason Smith [:jsmith] 2012-04-26 20:48:26 PDT
(In reply to Felipe Gomes (:felipe) from comment #10)
> (In reply to Jason Smith [:jsmith] from comment #9)
> > On OS X 10.6 - Does not fully work either. If I have the applications folder
> > open, and I install an application, the icon may not appear. If close
> > finder, reopen the applications directory, then I will see the icon.
> 
> Was this with a hourly mozilla-central build from this afternoon?

It was with today's nightly build (4/26/2012) on OS X 10.5/10.6/10.7. Ah I see the other patch will land tomorrow in mozilla in-bound. I'll move this back to resolved fixed and retest this tomorrow.
Comment 12 :Felipe Gomes (needinfo me!) 2012-04-26 20:57:51 PDT
Actually the relevant patches landed in mozilla-central today, so they will be in Nightly's tomorrow (the ones marked as 2012/04/27)
Comment 13 Jason Smith [:jsmith] 2012-04-30 17:11:34 PDT
Re-tested:

- OS X 10.5 - Fail (No icon shows up when installing an app - see bug 745999)
- OS X 10.6 - Pass
- OS X 10.7 - Pass

Should we reopen this bug for the issue in OS X 10.5? Or is the issue with OS X 10.5 separate from this bug, so we should track it in bug 745999?
Comment 14 :Felipe Gomes (needinfo me!) 2012-04-30 17:19:45 PDT
this bug was for a very specific issue, so let's track that in bug 745999
Comment 15 Jason Smith [:jsmith] 2012-04-30 17:20:34 PDT
(In reply to Felipe Gomes (:felipe) from comment #14)
> this bug was for a very specific issue, so let's track that in bug 745999

Sounds good. Marking this as verified. Followup work for the specific issue in bug 745999 will be tracked in that bug.

Note You need to log in before you can comment on or make changes to this bug.