Project

General

Profile

Bug #856484

Gtk3.22: city dialog emergency colour highlighting stops working after city dialog closed and opened again

Added by Jacob Nevins 6 months ago. Updated 5 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
gui-gtk-3.22
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:

Description

Various bits of the city dialog get coloured red by city_dialog_update_information() if something bad is going to happen.

However, in the Gtk3.22 client, this only works the first time the city dialog is opened. If the city dialog is closed and then opened again, emergencies aren't highlighted -- everything remains black regardless.

I'm guessing this is due to the different approach compared to other Gtk clients. There's a static variable holding a GtkCssProvider, initialised with gtk_css_provider_new() the first time it matters.

I'm guessing the lifetime management is wrong and this thing goes out of scope somehow when the city dialog is destroyed, so the static emergency_provider is then either useless or invalid.

(Seen on S2_6 and master. Checked Gtk3 is unaffected on both branches.)

26_gtkcssprovider_lifetime.patch (1.22 KB) 26_gtkcssprovider_lifetime.patch experimental logging patch to try to determine GtkCssProvider lifetime Jacob Nevins, 2020-01-26 12:32 PM
m-gtk322-emergency-provider.patch (6.61 KB) m-gtk322-emergency-provider.patch master (Gtk4 not tested) Jacob Nevins, 2020-01-26 12:58 PM
30-26-gtk322-emergency-provider.patch (3.6 KB) 30-26-gtk322-emergency-provider.patch S3_0, S2_6 Jacob Nevins, 2020-01-26 12:58 PM
m-gtk322-emergency-provider-bis.patch (10.2 KB) m-gtk322-emergency-provider-bis.patch master, improved version (gtk4 still untested) Jacob Nevins, 2020-01-26 01:18 PM
30-26-gtk322-emergency-provider-bis.patch (5.41 KB) 30-26-gtk322-emergency-provider-bis.patch S3_0/S2_6, improved version Jacob Nevins, 2020-01-26 01:19 PM

History

#1 Updated by Jacob Nevins 6 months ago

I can't find any documentation about the lifetime of GtkCssProvider objects.

Experimentally, the problem doesn't seem to be that the GtkCssProvider is destroyed with the city dialog, or anything like that... as far as I can tell it hangs around. It's not a floating reference, and adding a g_object_weak_ref I don't see evidence of it being destroyed when the city dialog is closed (or indeed ever).

#2 Updated by Jacob Nevins 6 months ago

  • Status changed from New to In Progress
  • Assignee set to Jacob Nevins

...oh, but, duh, it's only being added to the widgets that exist when the static GtkCssProvider is first created. The second time round, with fresh widgets, the GtkCssProvider isn't added to them (so the "emergency" style does nothing).

The fix is trivial. (This general approach seems a bit fussy -- we could probably make the "emergency" style available for every widget -- but I'm not going to make a big change now.)

#3 Updated by Jacob Nevins 6 months ago

Reviewing other similar code: detached_widget_fill() already gets this right. update_turn_done_button() does not, but the turn-done button is probably never destroyed.

#5 Updated by Jacob Nevins 6 months ago

...although now I'm worried that gtk_style_context_add_provider() will be called too often; I don't know if it's idempotent, or will consume some extra resource on a persistent city dialog each time it's called.

#7 Updated by Jacob Nevins 5 months ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF