Update to fix badge render issue

[gcasa]

Centralize dock tile rendering so that recent changes for image updates are consistent between docktiles and miniwindows.

[rmottola]

Unfortunately it doesn't work for me. My test is running GNUMail and GWorkspace dock (icn manager support branch).
My old specific test app does work again though (was broken yesterday) but it displays the badge only in the WindowMaker mini-window, not the dock.
It could be an issue with GWorkspace's Dock.

[rmottola]

in DockWM docks show, but because they are re-drawn and look differently.

[rmottola]

Now badges display inside GWorkspace's Dock! that is cool.

I think now we need to be sure of the code and coordinate all settings/preferences.

[rmottola]

I wonder if we really need the retry-timer, I'm always weary for such retries. Perhaps if it fails, just let the next update fix it?
Just thinking if the next update perhaps is only the next update the badge content changes or if there is an earlier event to piggy-back.

[gcasa]

I wonder if we really need the retry-timer, I'm always weary for such retries. Perhaps if it fails, just let the next update fix it? Just thinking if the next update perhaps is only the next update the badge content changes or if there is an earlier event to piggy-back.

It seemed the only way to solve the problem if the update is missed. Leaving it to the next change is going to be interpreted as a bug because the user doesn't see the current status.

[rmottola]

I wonder if we really need the retry-timer, I'm always weary for such retries. Perhaps if it fails, just let the next update fix it? Just thinking if the next update perhaps is only the next update the badge content changes or if there is an earlier event to piggy-back.

It seemed the only way to solve the problem if the update is missed. Leaving it to the next change is going to be interpreted as a bug because the user doesn't see the current status.

yes understandable, next change is not feasible, but perhaps there is another update event. It was a guess. Maybe @rfm has an idea.

As I read the code I see the Timer is always there, maybe it oculd be created only on failure.

[gcasa]

I have added @rfm as a reviewer. Please take a look and give some advice on this change.

[rmottola]

For me it s fine, I see no easy alternative to the retry timer. @rfm your take? green light?

[rmottola]

Fine for me.

[gcasa]

@fredkiefer do you have any issues before I merge this??

[fredkiefer]

Could you please wait with merging? You will also need to remove the conflicts first.
Could you explain whether this is a regression? I am currently swamped with PRs and newly reported issue and it really doesn't help when somebody additionally pings me on one. It would help if you went through the open issues and PRs and flag regressions and whether we should merge them.

[gcasa]

Could you please wait with merging? You will also need to remove the conflicts first. Could you explain whether this is a regression? I am currently swamped with PRs and newly reported issue and it really doesn't help when somebody additionally pings me on one. It would help if you went through the open issues and PRs and flag regressions and whether we should merge them.

It's a regression. The original change broke badge support in GWorkspace. Sorry, I realize you must be inundated.

[fredkiefer]

It's a regression. The original change broke badge support in GWorkspace. Sorry, I realize you must be inundated.

What do you mean by [regression]? I can see that the last change affected the default value and this PR is changing it back to the previous value. Any PR that has just this change would fix a regression. But here much more was done and I am not sure, whether the rest of this PR should really go in.

I am assuming you're referring to the timer that compensates for the badge being set prior to the application starting. This caused an issue where a badge should be set, but wasn't.

Restoring the default lets the application communicate with the dock again. The other changes address what happens after that: making sure badges are drawn and refreshed, preserving the image when it is copied, retrying updates the dock may miss during startup or restart, and handling docks that > don’t support every method.

Restoring the default fixes the regression. The additional changes address separate problems that can still leave badges missing or outdated.

Decomposing this PR into separate components complicates the issue and will only leave us open to the issue the change described above was meant to correct. @rmottola do you have anything to add?

[gcasa]

@fredkiefer I meant to quote reply on that last message.

[gcasa]

@fredkiefer I am not sure how that got changed back. I don't remember changing it so that the default was YES by default. I have changed it back.

[fredkiefer]

@fredkiefer I meant to quote reply on that last message.

Yes, you got me confused there :-)

Just to make things more clear. I am not against this change, but I am a bit confused with all the different changes for app icons. I understand that we need some of the changes from this PR to get back to behaviour that was present in the last release. What we should avoid is introducing other changes that break stuff for users. If we do so, we will keep on fixing regressions and never have a release.

[gcasa]

I just realized that the check handles both labels being nil. Without it, “no badge” looks like a change every time, causing unnecessary reconnect attempts when the icon manager isn’t available. I just tested it, and I'm changing it back. Sorry for all of the noise.

[rmottola]

Decomposing this PR into separate components complicates the issue and will only leave us open to the issue the change described above was meant to correct. @rmottola do you have anything to add?

Not much to add, decomposing this PR doesn't help. It addresses multiple issues on the already committed dock icon code. The timer stuff is needed if to address application vs. dock startup issue. The badge refactor code is needed so that the same badge drawing code is needed for dock icon and app icons. Richard asked to keep the default off for the GSUseIconManager as it was before.
I wouldn't thus question the scope and substance, but concentrate on cleaning up the code.
I should also probably retest the branch too. I had issues when I tried to rebase it the other day and didn't commit it and have seen that Gregory also needed a couple of attempts.

[rmottola]

Still works for me, just tested.

[fredkiefer]

I really would like to get this PR finished. Let's just say I merge what ever state this is in later today. In about five hours time, if this is fine for you.

[gcasa]

I really would like to get this PR finished. Let's just say I merge what ever state this is in later today. In about five hours time, if this is fine for you.

@fredkiefer That's fine with me if you're okay with the quality of the code. Also, I have tested it locally with the change you just suggested and it works fine. Also, @rmottola, please try yours again.

[gcasa]

Without the timer, the implementation in GWorkspace is not working. While this could be interpreted as a bug in GWorkspace, I think that the code in libs-gui should be more robust. What I am going to do is make it send the badge again after NSApplicationDidFinishLaunching (I believe that's the right notification).

BadgeTest is the app the second to last application at the bottom. It should have a 7 on it, but it's not.

[gcasa]

This fix was in fact the driving reason to do this PR... so without it working properly, there is no point to the change.

[fredkiefer]

This fix was in fact the driving reason to do this PR... so without it working properly, there is no point to the change.

I never asked you to remove the retry mechanism completely, I just wanted it to stop after some attempts and not to go on for ever.

[gcasa]

This fix was in fact the driving reason to do this PR... so without it working properly, there is no point to the change.

I never asked you to remove the retry mechanism completely, I just wanted it to stop after some attempts and not to go on for ever.

I will add it back in a limited sense, but after further testing I am strongly of the opinion @rmottola that this change maks a race condition in GWorkspace. GWorkspace is accepting the initial badge update and then dropping it on the floor. This is why hitting "Apply" will cause it to show because it forces an update to be sent. I will implement this so it retries a limited number of times (maybe 3) to get this to show as I still believe the gui code should be tolerant, but it shouldn't compensate or mask actual issues in the code.

This is a GWorkspace issue, I will add a PR there to fix it, but make the change I described above as well. This way we are covered on both ends.

[gcasa]

Now it retries after 1 second, then twice at 10-second intervals, and stops. Changing the badge resets the limit; clearing it cancels retries. This fixes the problem. See the screenshot:

[gcasa]

I think things with this are more reasonable now. We have a retry that is limited and the code has been cleaned up significantly thanks to @fredkiefer. :) So I am going to leave this PR alone. Pending any additional issues discovered by @rmottola might report. I have tested everything I know and it works just fine.

[fredkiefer]

I will merge it now and hope this fits for the release

[rmottola]

@fredkiefer @gcasa I did a quick test on the branch. GNUMail badge works fine in Workspace and Clock continues to display fine too. I only have here the raspberry, but fine. Further tests will follow in continuous day-by-day usage of course.