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 699224 - file chooser does not respect 12/24-hour clock setting
file chooser does not respect 12/24-hour clock setting
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkFileChooser
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
Federico Mena Quintero
filechooser-patch-review-needed
Depends on:
Blocks:
 
 
Reported: 2013-04-29 13:48 UTC by Adam Dingle
Modified: 2014-01-24 23:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Use GDateTime in the file chooser (4.26 KB, patch)
2013-07-12 15:06 UTC, Garrett Regier
needs-work Details | Review
Bug 699224 - Respect 12/24-hour clock setting in the file chooser (3.65 KB, patch)
2013-07-12 15:06 UTC, Garrett Regier
none Details | Review
Use GDateTime in the file chooser v2 (4.22 KB, patch)
2013-12-07 20:45 UTC, Garrett Regier
accepted-commit_now Details | Review
Bug 699224 - Respect 12/24-hour clock setting in the file v2 (3.26 KB, patch)
2013-12-07 20:46 UTC, Garrett Regier
needs-work Details | Review

Description Adam Dingle 2013-04-29 13:48:59 UTC
I'm running GTK from git master.

I have LANG=en_US.UTF-8, i.e. the U.S. which uses 12-hour time (AM/PM).  And I have /org/gnome/desktop/interface/clock-format = "12h".  Despite this, GtkFileChooser displays each file's modified date using 24-hour time (e.g. "16:18").  Nautilus is correctly displaying times with AM/PM.
Comment 1 Garrett Regier 2013-07-12 15:06:31 UTC
Created attachment 249017 [details] [review]
Use GDateTime in the file chooser

Much simpler, still UTF-8, and always has some GNU extensions available.
Comment 2 Garrett Regier 2013-07-12 15:06:51 UTC
Created attachment 249019 [details] [review]
Bug 699224 - Respect 12/24-hour clock setting in the file chooser
Comment 3 Timothy Arceri 2013-09-15 22:26:12 UTC
patch no longer seems to apply cleanly to master
Comment 4 Matthias Clasen 2013-09-17 02:24:20 UTC
Review of attachment 249017 [details] [review]:

::: gtk/gtkfilechooserdefault.c
@@ +3968,2 @@
     format = _("%H:%M");
+  else if (time_y == now_y && time_m == now_m && ABS (time_d - now_d) == 1)

I don't think this is quite correct. What if now == new years day, and time == new years eve ? The text should say 'Yesterday ...', yet time_y will be != now_y.
Comment 5 Garrett Regier 2013-12-07 20:45:45 UTC
Created attachment 263723 [details] [review]
Use GDateTime in the file chooser v2
Comment 6 Garrett Regier 2013-12-07 20:46:04 UTC
Created attachment 263724 [details] [review]
Bug 699224 - Respect 12/24-hour clock setting in the file v2
Comment 7 Matthias Clasen 2013-12-08 05:43:12 UTC
Review of attachment 263723 [details] [review]:

looks good now. The commit message could use some more detail
Comment 8 Matthias Clasen 2013-12-08 05:49:35 UTC
Review of attachment 263724 [details] [review]:

::: gtk/gtkfilechooserdefault.c
@@ +7500,3 @@
+                                       "org.gnome.desktop.interface",
+                                       TRUE) != NULL)
+    priv->interface_settings = g_settings_new ("org.gnome.desktop.interface");

I'm not convinced that we want to start sprinkling gnome settings dependencies over the gtk tree like this. Maybe a better way to go would be to add a 12/24 h setting to org.gtk.Settings.FileChooser, like we recently did for sort-directories-first.

We can then make the datetime panel set that too
Comment 9 Garrett Regier 2013-12-08 09:16:52 UTC
(In reply to comment #7)
> Review of attachment 263723 [details] [review]:
> 
> looks good now. The commit message could use some more detail

Anything in particular? It really just replaces the old convoluted code with the equivalent GDateTime code.


(In reply to comment #8)
> Review of attachment 263724 [details] [review]:
> 
> ::: gtk/gtkfilechooserdefault.c
> @@ +7500,3 @@
> +                                       "org.gnome.desktop.interface",
> +                                       TRUE) != NULL)
> +    priv->interface_settings = g_settings_new ("org.gnome.desktop.interface");
> 
> I'm not convinced that we want to start sprinkling gnome settings dependencies
> over the gtk tree like this. Maybe a better way to go would be to add a 12/24 h
> setting to org.gtk.Settings.FileChooser, like we recently did for
> sort-directories-first.
> 
> We can then make the datetime panel set that too

I specifically check to see if the schema exists to avoid having a hard dep, but just let me know how you want it implemented and I will modify it.
Comment 10 Matthias Clasen 2013-12-09 14:43:31 UTC
(In reply to comment #9)
> (In reply to comment #7)
> > Review of attachment 263723 [details] [review] [details]:
> > 
> > looks good now. The commit message could use some more detail
> 
> Anything in particular? It really just replaces the old convoluted code with
> the equivalent GDateTime code.

That would already be a good sentence to put there: 'Replace old convoluted code with the equivalent GDateTime code' Also worth mentioning that GDateTime has an API to compute the difference between to times, which we are using here.

Finally worth stating that this should not change the behavior of the code at
all.

> 
> (In reply to comment #8)
> > Review of attachment 263724 [details] [review] [details]:
> > 
> > ::: gtk/gtkfilechooserdefault.c
> > @@ +7500,3 @@
> > +                                       "org.gnome.desktop.interface",
> > +                                       TRUE) != NULL)
> > +    priv->interface_settings = g_settings_new ("org.gnome.desktop.interface");
> > 
> > I'm not convinced that we want to start sprinkling gnome settings dependencies
> > over the gtk tree like this. Maybe a better way to go would be to add a 12/24 h
> > setting to org.gtk.Settings.FileChooser, like we recently did for
> > sort-directories-first.
> > 
> > We can then make the datetime panel set that too
> 
> I specifically check to see if the schema exists to avoid having a hard dep,
> but just let me know how you want it implemented and I will modify it.

As I said: put a 12/24h preference in the org.gtk.settings.FileChooser schema, use that in the filechooser code, then file a bug against gnome-control-center to set this together with the other setting