Created attachment 218784 [details] [review]
window: Handle changes of the attach-modal-dialogs preference

Currently we decide whether a modal dialog should be attached or not
when mapping it, i.e. we don't pick up preference changes that happen
while the dialog is up. It's not really a big deal given that modal
dialogs are usually transitory, but it's easy enough to add a bit of
extra polish ...

Review of attachment 218784 [details] [review]:

Patch looks fine, but it's a minor fix for a bug that was there since 3.0 and nobody ever noticed it, except after staring at the code. I do not honestly think it's worth it to correct this.

The main issue is that we're still doing this "prefs changed callback" system that carries over from GConf, instead of embracing GSettings and connecting to a signal.

Marking as "reviewed" as it's sort of a half ACN, half "this code sucks, but it's not your fault, and nobody has time to correct it", and half "is this really necessary".

(In reply to comment #2)
> The main issue is that we're still doing this "prefs changed callback" system
> that carries over from GConf, instead of embracing GSettings and connecting to
> a signal.

I think the reason for the listener system is rather that there is no MetaPrefs object which could emit a signal :-)

(connecting directly to the various GSettings is not a good idea as those are potentially overridden)

(In reply to comment #2)
> Review of attachment 218784 [details] [review]:
> 
> Patch looks fine, but it's a minor fix for a bug that was there since 3.0 and
> nobody ever noticed it, except after staring at the code. I do not honestly
> think it's worth it to correct this.

Basically as long as Florian found it worthwhile to come up with a patch, I think it's worthwhile to apply it - even if we were to switch over to signal connections on GSettings later, it's useful to know what code we need.

Attachment 218784 [details] pushed as eb1292e - window: Handle changes of the attach-modal-dialogs preference