After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 695807 - [PATCH] Calendar window doesn't remember location expander state
[PATCH] Calendar window doesn't remember location expander state
Status: RESOLVED FIXED
Product: gnome-panel
Classification: Other
Component: clock
git master
Other Linux
: Normal normal
: ---
Assigned To: Panel Maintainers
Panel Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-03-13 22:31 UTC by Jethro Beekman
Modified: 2015-03-24 13:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Make calendar window remember expander state (421 bytes, patch)
2013-03-13 22:31 UTC, Jethro Beekman
committed Details | Review
Make calendar window remember expander state (980 bytes, patch)
2013-10-24 09:51 UTC, Sebastian
rejected Details | Review

Description Jethro Beekman 2013-03-13 22:31:36 UTC
Created attachment 238830 [details] [review]
Make calendar window remember expander state

Somewhere in the switch from gconf to gsettings, the state for the location expander got lost.

Even if gnome-panel is compiled without libecal, the location window should still remember its state. The #ifdef in create_hig_frame is not needed because it only gets called from functions that have the #ifdef or from the location expander functions, which do need to settings binding.
Comment 1 Sebastian 2013-10-09 23:01:35 UTC
Marking as NEW since I can verify this bug. Unfortunately I cannot verify the patch right now. But it looks logical to me. Maybe Philipp or Alberts can make a comment?
Comment 2 Stefano Karapetsas 2013-10-09 23:26:14 UTC
Sebastian,
I can confirm this issue was fixed in a similar way in MATE, also if it was with GConf/MateConf:

http://git.mate-desktop.org/mate-panel/commit/?id=f27accd
Comment 3 Sebastian 2013-10-24 08:17:43 UTC
Review of attachment 238830 [details] [review]:

The patch works and can be commited.
Comment 4 Philipp 2013-10-24 09:33:52 UTC
Review of attachment 238830 [details] [review]:

If we change this, let's do it right. :)

::: applets/clock/calendar-window.c
@@ +1534,1 @@
 	g_settings_bind (calwin->priv->settings, key, expander, "expanded",

Could you use the KEY_LOCATIONS_EXPANDED constant here ?
Comment 5 Sebastian 2013-10-24 09:51:02 UTC
Created attachment 257995 [details] [review]
Make calendar window remember expander state

Sure, here is the new patch.
Comment 6 Philipp 2013-10-24 17:52:26 UTC
Review of attachment 257995 [details] [review]:

::: applets/clock/calendar-window.c
@@ +1533,3 @@
 
+	g_settings_bind (calwin->priv->settings, key, expander,
+                     KEY_LOCATIONS_EXPANDED,

Thinko on my part; KEY_LOCATIONS_EXPANDED is actually passed into the function and is the "key" variable here.
Comment 7 Philipp 2013-10-24 18:17:54 UTC
Review of attachment 238830 [details] [review]:

Patch works for me partially; however, if I actually try to add a location outside of the US, the clock applet crashes. As I can reproduce that crash without this patch, I guess this is an existing regression. I'll see if a big has already been filed about this.

Therefore, OK'ed and commited to gnome-3-8 in 18aa7a9171f9c4b3462d5bb189249281750d0def and merged to master in d877f003271eb5d49c363abe4c2cfe7cf2884051.

::: applets/clock/calendar-window.c
@@ +1534,1 @@
 	g_settings_bind (calwin->priv->settings, key, expander, "expanded",

Sorry, code was correct after all.