Merge lp:~gang65/ubuntu-clock-app/ubuntu-clock-app-australia-display-fix into lp:ubuntu-clock-app

[Bartosz Kosiorek]

Fix digital time display for English Australia locale (LP: #1384739)

Fix digital time display for English Australia locale (LP: #1384739)

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:398
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/851/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/196

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/851/rebuild

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

2 inline comments.

review: Needs Information

[Nekhelesh Ramananthan (nik90)]

As per the inline comments, I do think that this is a upstream QT issue IMO. I also commented about it in the bug report at https://bugs.launchpad.net/ubuntu-clock-app/+bug/1384739/comments/5. By patching it downstream in the clock app, I fear we might break other locales without proper testing.

Personally I don't like introducing downstream patches when the correct solution breaks in certain use-cases due to upstream bugs.

review: Disapprove

[Bartosz Kosiorek (gang65)]

Ok. I understand that it needs extensive testing.

Can we move this patch to the next Clock release then?

review: Needs Information

[Nekhelesh Ramananthan (nik90)]

> Ok. I understand that it needs extensive testing.
>
> Can we move this patch to the next Clock release then?

It would be better if the fix for this issue came from QtUbuntu [1] rather than patching it in the clock app. Downstream QT Patches are done in QtUbuntu (and upstream after a while). Can you talk to Timo Jyrinki who usually does this? You can find him in #ubuntu-app-devel, #ubuntu-touch etc under the nickname Mirv.

Let's not add these kind of patches in clock app due to upstream issues *unless* it is a High/Critical issue that affects a large portion of users. Instead we should convince upstream to fix these issues.

Testing is just one aspect of it. It just adds the headache of maintaining that patch ourselves once we add it.

[1] https://launchpad.net/qtubuntu

[Nekhelesh Ramananthan (nik90)]

I have updated the bug report to reflect the decision and the way to move forward with this issue.

Unmerged revisions

398. By Bartosz Kosiorek on 2015-10-12

Fix digital time display for English Australia locale (LP: #1384739)

=== modified file 'app/components/DigitalMode.qml'
--- app/components/DigitalMode.qml	2015-09-16 15:13:36 +0000
+++ app/components/DigitalMode.qml	2015-10-12 20:43:31 +0000
@@ -56,17 +56,20 @@
         color: UbuntuColors.midAubergine
         font.pixelSize: units.dp(1)
         text: {
-            if (localizedTimeString.search(Qt.locale().amText) !== -1) {
-                // 12 hour format detected with the localised AM text
-                return localizedTimeString.replace(Qt.locale().amText, "").trim()
-            }
-            else if (localizedTimeString.search(Qt.locale().pmText) !== -1) {
-                // 12 hour format detected with the localised PM text
-                return localizedTimeString.replace(Qt.locale().pmText, "").trim()
-            }
-            else {
-                // 24-hour format detected, return full time string
-                return localizedTimeString
+            // Fix for lp:1384739
+            if (localizedTimeString.split(" ").length > 1) {
+                localizedTimeString.split(" ")[0]
+            } else {
+                if (localizedTimeString.search(Qt.locale().amText) !== -1) {
+                    // 12 hour format detected with the localised AM text
+                    return localizedTimeString.replace(Qt.locale().amText, "").trim()
+                } else if (localizedTimeString.search(Qt.locale().pmText) !== -1) {
+                    // 12 hour format detected with the localised PM text
+                    return localizedTimeString.replace(Qt.locale().pmText, "").trim()
+                } else {
+                    // 24-hour format detected, return full time string
+                    return localizedTimeString
+                }
             }
         }
     }
@@ -81,17 +84,20 @@
         font.pixelSize: units.dp(1)
         visible: text !== ""
         text: {
-            if (localizedTimeString.search(Qt.locale().amText) !== -1) {
-                // 12 hour format detected with the localised AM text
-                return Qt.locale().amText
-            }
-            else if (localizedTimeString.search(Qt.locale().pmText) !== -1) {
-                // 12 hour format detected with the localised PM text
-                return Qt.locale().pmText
-            }
-            else {
-                // 24-hour format detected
-                return ""
+            // Fix for lp:1384739
+            if (localizedTimeString.split(" ").length > 1) {
+                localizedTimeString.split(" ")[1] 
+            } else {
+                if (localizedTimeString.search(Qt.locale().amText) !== -1) {
+                    // 12 hour format detected with the localised AM text
+                    return Qt.locale().amText
+                } else if (localizedTimeString.search(Qt.locale().pmText) !== -1) {
+                    // 12 hour format detected with the localised PM text
+                    return Qt.locale().pmText
+                } else {
+                    // 24-hour format detected
+                    return ""
+                }
             }
         }
     }

=== modified file 'debian/changelog'
--- debian/changelog	2015-09-17 04:33:11 +0000
+++ debian/changelog	2015-10-12 20:43:31 +0000
@@ -16,6 +16,7 @@
   * Fix stopwatch issue appering during changing timezone during runtime (LP: #1493358)
   * Fix Daylight Saving Time issues (LP: #1437805)
   * Fix time for second location wrong after daylight saving started (LP: #1457523)
+  * Fix digital time display for English Australia locale (LP: #1384739)
 
  -- Bartosz Kosiorek <[e-mail]>  Wed, 02 Sep 2015 15:16:29 +0200