search

gnustep / libs-back

The GNUstep gui library is a library of graphical user interface classes written completely in the Objective-C language; the classes are based upon Apple's Cocoa framework (which came from the OpenStep specification). *** Larger patches require copyright assignment to FSF. please file bugs here. ***
http://www.gnustep.org
GNU General Public License v3.0
50 stars 34 forks source link

Fix for app icon problem. #2

Closed gcasa closed 6 years ago

gcasa commented 6 years ago

Please review these changes. The issue was that when we get the icon we also initialize a window which causes an exception since the backend tries to pull all screens before everything is initialized. Since all we are after is the icon so it can be stored I have separated these into separate methods.

fredkiefer commented 6 years ago

When I first looked at the code I thought that this change would belong into gui and be equivalent to rewriting the method applicationIconImage to be something like this:

But looking into _appIconInit showed that this won't set the value of _app_icon. So it seems like the whole logic here is screwed. The result of the second call to applicationIconImage should again be nil. If this change actually solves the problem then it is by coincidence and we should try to better understand this. In NSApplication is already a call to _appIconInit in line 924. Is this too early or too late?

gcasa commented 6 years ago

The call at 924 is too late. The _createAppIconPixmaps method is called when the backend is initialized in the beginning of the _init method in NSApplication. So, by this time, it's already too late to add the app icon so it can be referenced by windowmaker. I separated getting the icon from setting it on the icon window so that we could have it set before the backend was initialized and the _createAppIconPixmaps method was called.

The previous changes (combining atoms) apparently changed the order in which these methods were called.

fredkiefer commented 6 years ago

Are you sure about the call at line 924 being too late? From looking at the code I think _createAppIconPixmaps is only called when we order the icon window in. Which should be a lot later. Could you please verify this by adding log statements before line 924 and in the _createAppIconPixmaps method?

gcasa commented 6 years ago

Yes, if you take out the call that gets the image or run the code on master it doesn't get the image in time to save the images for use by WindowMaker. The problem here is only that the icon is not showing in the dock when the app isn't active.

fredkiefer commented 6 years ago

Could you please verify this with a few log statements?

gcasa commented 6 years ago

It appears as though I forgot the gui portion of this change.

gcasa commented 6 years ago

Actually, this change may not be needed after all... I just tested with libs-back master and my changes for libs-gui and it works as expected.

fredkiefer commented 6 years ago

OK, so let us close this pull request and focus on the other one.

gcasa commented 6 years ago

Closing since the changes here are superfluous