Bug 335050 - Time shown by clock does not take timezone into account
Summary: Time shown by clock does not take timezone into account
Status: RESOLVED FIXED
Alias: None
Product: plasmashell
Classification: Plasma
Component: Digital Clock (show other bugs)
Version: master
Platform: Exherbo Linux
: NOR normal
Target Milestone: 1.0
Assignee: Martin Klapetek
URL:
Keywords:
: 335951 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-05-19 20:14 UTC by Bernd Steinhauser
Modified: 2014-06-11 13:59 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
digital clock in a hurry (27.37 KB, image/png)
2014-05-22 20:06 UTC, Alex Minton
Details
QTimeZone testapp (1.18 KB, application/zip)
2014-06-05 16:41 UTC, Martin Klapetek
Details
Snapshot of plasmaengineexplorer output (115.38 KB, image/png)
2014-06-09 20:56 UTC, AE
Details
Minimal example to trigger the timezone bug (1.67 KB, text/plain)
2014-06-10 01:35 UTC, AE
Details
Fix for the timezone bug. (1.08 KB, patch)
2014-06-10 01:58 UTC, AE
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Bernd Steinhauser 2014-05-19 20:14:56 UTC
I chose digital clock, but I can see this with the analog clock, too.
My timezone is CEST and when I tell the clock to show the timezone, it correctly shows CEST. Output by `date` is the correct time (currently GMT+2), but the time shown by the clock is the system time (UTC, GMT).

Reproducible: Always

Steps to Reproduce:
1. Start plasmashell, ensure that a clock item (analog or digital) is shown
2. Ensure that timezone is not set to GMT
3. Compare the output of the clock widget and `date`
Actual Results:  
Clock widget shows GMT time.

Expected Results:  
Clock widget should show the time according to the current timezone setting.
Comment 1 Martin Klapetek 2014-05-19 20:22:41 UTC
Thanks for the report!

How are you setting your timezone (and making sure it's set)? Also can you please give me output of "env | grep LC"

Thanks
Comment 2 Bernd Steinhauser 2014-05-20 05:42:59 UTC
The timezone is set by a symlink:
/etc/localtime -> /usr/share/zoneinfo/Europe/Berlin

env | grep LC returns nothing.

I think that is the most common technique these days and is also what timedatectl from systemd does. Changing that does actually make a difference, so it is getting weird now.
I changed the timezone to GMT and the clock showed that the timezone is now GMT (before it showed CEST). Changed it back and the timezone did change back, but it also set the correct time now.
So there might be a problem with the initial setup of the time?
Comment 3 Sebastian Kügler 2014-05-20 11:36:22 UTC
Could you also check the value of the TZ env var?
Comment 4 Bernd Steinhauser 2014-05-20 15:55:16 UTC
There is no TZ env var set.
Comment 5 Martin Klapetek 2014-05-21 08:36:58 UTC
Ok so your timezone is now correct, right? Do you have a way to reproduce that state? We use "QTimeZone::systemTimeZoneId()" for the current timezone and that uses many ways to make sure it has the correct timezone. I have seen it broken if your locale is set to C though (then the timezone is GMT).

Which distro are you running btw?
Comment 6 Alex Minton 2014-05-22 20:06:48 UTC
Created attachment 86779 [details]
digital clock in a hurry

I have a similar problem with digital clock. The timezone showed by it is correct, but the time itself is one hour ahead of my local time (e.g. digital clock is one hour ahead of "date" output)
Comment 7 Martin Klapetek 2014-05-23 08:34:28 UTC
Which distribution are you running that on?
Comment 8 Alex Minton 2014-05-23 09:04:56 UTC
I'm on openSUSE 13.1
Comment 9 Bernd Steinhauser 2014-05-23 17:33:49 UTC
(In reply to comment #5)
> Ok so your timezone is now correct, right? Do you have a way to reproduce
> that state? We use "QTimeZone::systemTimeZoneId()" for the current timezone
> and that uses many ways to make sure it has the correct timezone.
Actually, I can't. The symlink is exactly the same as before. There are no env vars referring to the timezone or GMT. I don't know what was causing it, but it's gone.

> I have
> seen it broken if your locale is set to C though (then the timezone is GMT).
No, that's not the case over here.
 
> Which distro are you running btw?
Exherbo, as given in the Platform field above.

I still don't understand, what happened.
Comment 10 Martin Klapetek 2014-06-05 14:44:01 UTC
As there's no way to reproduce this bug, I'm closing this.

Please reopen if you find a way to reproduce that state with steps on how to reproduce; it's also possible this is a bug in QTimeZone.
Comment 11 David Edmundson 2014-06-05 15:58:34 UTC
I still have the issue.
My locale is now apparently right.
Comment 12 Martin Klapetek 2014-06-05 16:09:06 UTC
Can you give me the output of 

$ timedatectl status
$ sudo hwclock --debug
$ date
Comment 13 David Edmundson 2014-06-05 16:23:30 UTC
$ timedatectl status
      Local time: Thu 2014-06-05 18:22:44 CEST
  Universal time: Thu 2014-06-05 16:22:44 UTC
        RTC time: Thu 2014-06-05 16:22:44
       Time zone: n/a (CEST, +0200)
     NTP enabled: n/a
NTP synchronized: no
 RTC in local TZ: no
      DST active: yes
 Last DST change: DST began at
                  Sun 2014-03-30 01:59:59 CET
                  Sun 2014-03-30 03:00:00 CEST
 Next DST change: DST ends (the clock jumps one hour backwards) at
                  Sun 2014-10-26 02:59:59 CEST
                  Sun 2014-10-26 02:00:00 CET


$  sudo hwclock --debug
[sudo] password for david: 
Sorry, try again.
[sudo] password for david: 
hwclock from util-linux 2.24.1
Using /dev interface to clock.
Last drift adjustment done at 1388772877 seconds after 1969
Last calibration done at 1388772877 seconds after 1969
Hardware clock is on UTC time
Assuming hardware clock is kept in UTC time.
Waiting for clock tick...
...got clock tick
Time read from Hardware Clock: 2014/06/05 16:23:08
Hw clock time : 2014/06/05 16:23:08 = 1401985388 seconds since 1969
Thu 05 Jun 2014 18:23:08 CEST  -0.391118 seconds

$ date
Thu  5 Jun 18:23:17 CEST 2014


Run at ~18:23, plasma clock shows 16:23
I'm in UTC+2
Comment 14 Martin Klapetek 2014-06-05 16:41:45 UTC
Created attachment 87028 [details]
QTimeZone testapp

Ok, I think the

"Time zone: n/a (CEST, +0200)"

might be the problem. Can you please run the attached code and paste the output?
Comment 15 Martin Klapetek 2014-06-08 11:44:56 UTC
*** Bug 335951 has been marked as a duplicate of this bug. ***
Comment 16 AE 2014-06-08 12:48:42 UTC
Hi, coming from bug #335951. The 'tzinfo' utility returned
"Europe/London" true
Hope that helps...
Comment 17 Martin Klapetek 2014-06-09 09:39:28 UTC
Thanks; I'll need some additional data.

Please run plasmaengineexplorer (make sure it's the one coming from your kf5 env), select "time" and in the list that appears find "Local" (should be the last one), expand that and check if all the data in there are correct (posting a screenshot would also be cool).
Comment 18 Martin Klapetek 2014-06-09 09:51:11 UTC
Andreas - which distribution are you running btw?
Comment 19 AE 2014-06-09 16:18:02 UTC
I'm running gentoo. It seems I didn't build plasmaengineexplorer, as I can an only find the one from plasmate (git), which I think is still on kde-4 level. Which kf5 package contains the explorer?
Comment 20 Martin Klapetek 2014-06-09 16:25:34 UTC
Right; it's part of plasmate now, so please build that, branch frameworks (you can run just the cmake, then cd into build/engineexplorer and build only that folder, no need to install even).
Comment 21 AE 2014-06-09 20:56:43 UTC
Created attachment 87091 [details]
Snapshot of plasmaengineexplorer output

Ok, built plasmaengineexplorer as instructed and ran it; the output time there is the correct one.
Comment 22 Martin Klapetek 2014-06-09 21:09:10 UTC
Oh that's very interesting; thanks.

I looked again at the code and all it does is 

text: Qt.formatTime(dataSource.data["Local"]["Time"], main.timeFormat);

So it takes the value directly from the dataengine and passes it to Qt.formatTime(). Either that function is messing things up or there is a wrong conversion between C++ QTime and QML Date.

It doesn't happen on my machine however, so that makes debugging that a bit harder. I'd like to try some patches; can you ping me on irc sometime around 2 PM your time (mck182 in #plasma on freenode) if you're interested in helping me debug this, it would be really helpful.
Comment 23 AE 2014-06-10 01:35:25 UTC
Created attachment 87095 [details]
Minimal example to trigger the timezone bug

IRC tomorrow afternoon won't work for me, unfortunately, but I was able to isolate the bug; see the attachment for a brief explanation.
Comment 24 AE 2014-06-10 01:58:00 UTC
Created attachment 87096 [details]
Fix for the timezone bug.

Patchy patch. Maybe it would be a good idea to have just the "DateTime" object in the source and leave all formatting to the consumer.

Going off on a tangent here, but DigitalClock.qml says:
    // Qt's QLocale does not offer any modular time creating like Klocale did
    // eg. no "gimme time with seconds" or "gimme time without seconds and with timezone".
    // QLocale supports only two formats - Long and Short. Long is unusable in many situations
    // and Short does not provide seconds. So if seconds are enabled, we need to add it here.

Not true any more, there's a third mode now, NarrowFormat, which might be very useful in getting rid of all the timeFormatCorrection() stuff:
http://qt-project.org/doc/qt-5/qlocale.html#FormatType-enum
Comment 25 Martin Klapetek 2014-06-10 09:06:23 UTC
That's a perfect work, thank you very much Andreas; I owe you a beer :) That was pretty much what I wanted to try out.

> Maybe it would be a good idea to have just the "DateTime" object in the source and leave all formatting to the consumer.

The applet is the consumer and the applet does the formatting. Or what formatting you have in mind?

> Not true any more, there's a third mode now, NarrowFormat, which might be very useful in getting rid of all the timeFormatCorrection() stuff:

The purpose of the timeFormatCorrection() is for options "Show seconds" and "Show timezone", the NarrowFormat for time is pretty much the same as ShortFormat and neither provides seconds or timezone, therefore we need to add it ourselves if the users sets that option...and that's what timeFormatCorrection() does ;)
Comment 26 AE 2014-06-10 09:29:58 UTC
(In reply to comment #25)
> That's a perfect work, thank you very much Andreas; I owe you a beer :) 
Thanks, give a shout when you happen to be in Oxford. ;)

> > Maybe it would be a good idea to have just the "DateTime" object in the source and leave all formatting to the consumer.
> 
> The applet is the consumer and the applet does the formatting. Or what
> formatting you have in mind?
Returning an object via QDateTime::date() and QDateTime::time() already is kinda sorta pre-formatting, as it drops certain information (and mucks things up, as it turns out). Can't you in principle get any information you want out of the QDateTime object already with the right format string?

> > Not true any more, there's a third mode now, NarrowFormat, which might be very useful in getting rid of all the timeFormatCorrection() stuff:
> 
> The purpose of the timeFormatCorrection() is for options "Show seconds" and
> "Show timezone", the NarrowFormat for time is pretty much the same as
> ShortFormat and neither provides seconds or timezone, therefore we need to
> add it ourselves if the users sets that option...and that's what
> timeFormatCorrection() does ;)
I see...


Cheers,

Andreas
Comment 27 Martin Klapetek 2014-06-10 09:38:30 UTC
> Returning an object via QDateTime::date() and QDateTime::time() already is kinda sorta pre-formatting, as it drops certain information (and mucks things up, as it turns out). Can't you in principle get any information you want out of the QDateTime object already with the right format string?

Ah yes, that is true. This design is inherited from the KDE4 version of it and I guess it was meant as a kind of shortcut or maybe the KDE4 applet was actually needing it, I don't know.

But I agree we can just keep DateTime, remove the rest and use the Qt date/time formatter. I'll cook a patch.
Comment 28 Sebastian Kügler 2014-06-10 11:37:46 UTC
Hm, the Plasma 4-based version never made the transition to QML.
Comment 29 Martin Klapetek 2014-06-11 13:38:54 UTC
Git commit fa8c62b32a0473cbc70c36226fc731d6c1076183 by Martin Klapetek.
Committed on 11/06/2014 at 13:38.
Pushed by mklapetek into branch 'master'.

Fix time showing applets not showing correct time for some timezones

Apparently passing QDateTime::time() to the Qt.formatTime is messing
things up; might also be a Qt bug, on the other hand the docs for QTime
say that "Unlike QDateTime, QTime knows nothing about time zones or
daylight savings time (DST)".

Passing the full QDateTime object makes the applets show correct time in
all timezones.

Hats off to Andreas Eckstein for finding out this

M  +2    -2    applets/analog-clock/contents/ui/analogclock.qml
M  +2    -2    applets/digital-clock/package/contents/ui/DigitalClock.qml

http://commits.kde.org/plasma-workspace/fa8c62b32a0473cbc70c36226fc731d6c1076183
Comment 30 AE 2014-06-11 13:49:51 UTC
One more occurance I'm afraid, in the clock I get when I lock the screen. Grepping for '"Time"' points to this file as the offender:

/usr/share/plasma/look-and-feel/org.kde.lookandfeel/contents/components/InfoPane.qml
Comment 31 Martin Klapetek 2014-06-11 13:56:52 UTC
Git commit a7bf16f5bbca8f0767eef45af9bfdf6017f109d7 by Martin Klapetek.
Committed on 11/06/2014 at 13:56.
Pushed by mklapetek into branch 'master'.

Fix time also in the lock screen

M  +1    -1    lookandfeel/contents/components/InfoPane.qml

http://commits.kde.org/plasma-workspace/a7bf16f5bbca8f0767eef45af9bfdf6017f109d7
Comment 32 Martin Klapetek 2014-06-11 13:59:19 UTC
Fixed, thanks :)