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 756377 - 00:xx times displayed in 12hour clock mode
00:xx times displayed in 12hour clock mode
Status: RESOLVED FIXED
Product: gnome-calendar
Classification: Applications
Component: General
3.18.x
Other Linux
: Normal normal
: 3.26
Assigned To: GNOME Calendar maintainers
GNOME Calendar maintainers
Depends on:
Blocks:
 
 
Reported: 2015-10-11 05:59 UTC by Trevor Gale
Modified: 2017-04-17 18:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
A simple patch fixing this problem (1.11 KB, patch)
2015-10-11 05:59 UTC, Trevor Gale
none Details | Review
12h clock time-selector fix (1.63 KB, patch)
2015-10-12 23:02 UTC, Trevor Gale
needs-work Details | Review
time selector fix for 12h setting (1.76 KB, patch)
2015-10-17 04:44 UTC, Trevor Gale
needs-work Details | Review
time-selector: fix 12-hour format (2.95 KB, patch)
2016-07-27 22:17 UTC, Ernestas Kulik
committed Details | Review

Description Trevor Gale 2015-10-11 05:59:27 UTC
Created attachment 313045 [details] [review]
A simple patch fixing this problem

The 12hour clock in gnome-calendar displays the time "12:30pm" as "00:30pm" which is not the standard(at least that is not the standard here in north america). This patch fixes this in the edit box for both the entering and display of values.
Comment 1 Erick Perez Castellanos 2015-10-12 20:31:21 UTC
(In reply to Trevor Gale from comment #0)
> Created attachment 313045 [details] [review] [review]
> A simple patch fixing this problem
> 
> The 12hour clock in gnome-calendar displays the time "12:30pm" as "00:30pm"
> which is not the standard(at least that is not the standard here in north
> america). This patch fixes this in the edit box for both the entering and
> display of values.

If you post the patch properly formatted I can review it. A really fast look at it, tells me it will show 12:05AM too, and we want 00:05 AM, right?
Comment 2 Trevor Gale 2015-10-12 23:02:42 UTC
Created attachment 313150 [details] [review]
12h clock time-selector fix
Comment 3 Trevor Gale 2015-10-12 23:12:26 UTC
The gnome-calendar app currently(without the patch) shows the time 5 minutes after midnight as 00:05AM. The standard way of displaying that time is 12:05AM. The same thing happens 5 minutes after noon(00:05 becomes 12:05PM). 

Hopefully my re-submitted patch is correctly formatted, if it's not please let me know and I will try to fix the problem(whether it is with the submission method or coding style).
Comment 4 Erick Perez Castellanos 2015-10-13 15:33:42 UTC
(In reply to Trevor Gale from comment #3)
> The gnome-calendar app currently(without the patch) shows the time 5 minutes
> after midnight as 00:05AM. The standard way of displaying that time is
> 12:05AM. The same thing happens 5 minutes after noon(00:05 becomes 12:05PM). 
> 
> Hopefully my re-submitted patch is correctly formatted, if it's not please
> let me know and I will try to fix the problem(whether it is with the
> submission method or coding style).

I'll take a look tonight, tomorrow morning you'll have your answer.
Comment 5 Erick Perez Castellanos 2015-10-15 13:37:03 UTC
Review of attachment 313150 [details] [review]:

Improve it, and we'll push it

::: src/gcal-time-selector.c
@@ +143,3 @@
     gtk_adjustment_set_upper (gtk_spin_button_get_adjustment (GTK_SPIN_BUTTON (priv->hour_spin)), 23.0);
   else
+    gtk_adjustment_set_upper (gtk_spin_button_get_adjustment (GTK_SPIN_BUTTON (priv->hour_spin)), 12.0);

If you're going to set this, then we need to call GtkAdjustment.set_lower() with 1. Otherwise we'll have 13 hours from [0, 12] both included.

@@ +241,3 @@
+      if(hours == 0){
+        hours = 12;
+      }

I don't like the if, I rather have this:
    `hours = hours % 12` + (hours == 0 ? 12 : 0);`

It's not a big deal tho.
Comment 6 Erick Perez Castellanos 2015-10-15 13:38:08 UTC
(In reply to Trevor Gale from comment #3)
> The gnome-calendar app currently(without the patch) shows the time 5 minutes
> after midnight as 00:05AM. The standard way of displaying that time is
> 12:05AM. The same thing happens 5 minutes after noon(00:05 becomes 12:05PM). 
> 
> Hopefully my re-submitted patch is correctly formatted, if it's not please
> let me know and I will try to fix the problem(whether it is with the
> submission method or coding style).

Sorry, it took me more than what I expected to get to it. Thank you for your work on this.
Comment 7 Trevor Gale 2015-10-17 04:44:34 UTC
Created attachment 313511 [details] [review]
time selector fix for 12h setting
Comment 8 Trevor Gale 2015-10-17 04:46:11 UTC
I included the changes as you suggested.
Comment 9 Erick Perez Castellanos 2015-10-17 16:19:54 UTC
Review of attachment 313511 [details] [review]:

::: src/gcal-time-selector.c
@@ +144,3 @@
   else
+    gtk_adjustment_set_lower (gtk_spin_button_get_adjustment (GTK_SPIN_BUTTON (priv->hour_spin)), 1.0);
+    gtk_adjustment_set_upper (gtk_spin_button_get_adjustment (GTK_SPIN_BUTTON (priv->hour_spin)), 12.0);

You're missing two pairs of curly brackets here, right? Otherwise the else part of the if will just execute the first statement.

Then for code styling issues, if you put curly brackets in the else part, and you need them, you'll have to add them to the then part.
Comment 10 Ernestas Kulik 2016-07-27 22:17:45 UTC
Created attachment 332245 [details] [review]
time-selector: fix 12-hour format

Currently, the date selector allows hours in range [0; 11], which is not
the convention. This commit fixes the conversion logic.
Comment 11 Georges Basile Stavracas Neto 2016-07-28 19:42:04 UTC
Comment on attachment 332245 [details] [review]
time-selector: fix 12-hour format

Sure.
Comment 12 Georges Basile Stavracas Neto 2016-07-28 19:42:47 UTC
Attachment 332245 [details] pushed as c16864f - time-selector: fix 12-hour format