Closed
Bug 1061120
Opened 10 years ago
Closed 10 years ago
[Calllog][Dialer] Time duration format is not showing 'min' or 's' in 'Call Information'
Categories
(Firefox OS Graveyard :: Gaia::Dialer, defect)
Tracking
(blocking-b2g:2.1+, b2g-v2.1 verified, b2g-v2.2 verified)
People
(Reporter: ericcc, Assigned: davidg)
References
Details
(Keywords: late-l10n, Whiteboard: [planned-sprint c=3])
Attachments
(5 files)
34.52 KB,
image/png
|
Details | |
44.59 KB,
image/png
|
Details | |
46 bytes,
text/x-github-pull-request
|
drs
:
review+
bajaj
:
approval-gaia-v2.1+
|
Details | Review |
46 bytes,
text/x-github-pull-request
|
drs
:
review+
bajaj
:
approval-gaia-v2.1+
|
Details | Review |
46 bytes,
text/x-github-pull-request
|
Details | Review |
### STR 1. Have a connected call in call log more than 0 second. ### Actual E.G. 00:04 shown. 2014-09-01-16-20-32.png ### Expected 00 min 4 s Page 5/Call info view/https://bugzilla.mozilla.org/attachment.cgi?id=8463237 #version Gaia 2be78d83a760fa3b9638fe51c266b442d14597f1 Gecko https://hg.mozilla.org/mozilla-central/rev/1db35d2c9a2f BuildID 20140831160203 Version 34.0a1 ro.build.version.incremental=110 ro.build.date=Fri Jun 27 15:57:58 CST 2014 B1TC00011230
Reporter | ||
Comment 1•10 years ago
|
||
[Blocking Requested - why for this release]: feature not complete 00 min 4 s Page 5/Call info view/https://bugzilla.mozilla.org/attachment.cgi?id=8463237
blocking-b2g: --- → 2.1?
QA Whiteboard: [COM=Gaia::Dialer]
Reporter | ||
Comment 2•10 years ago
|
||
Hi Carrie, Got a question on the formatting: A call is more than 1 minute, shown as 1 min 5 s. A call is exactly 1 minute, shown as 1 min. A call is less than 1 minute, shown as 12 s. [Q] A call is exactly 0 minute, shown as??? 0 min 0 s? [Q] How about a call is more than 1 hour? 1hr 3 min 2 s?
Flags: needinfo?(cawang)
Updated•10 years ago
|
Whiteboard: [planned-sprint]
Target Milestone: --- → 2.1 S4 (12sep)
Comment 3•10 years ago
|
||
Whoever takes this should discuss this with the l10n team to see if we can share some code with l10n_date.js. See bug 1060355. If we decide against sharing, it'll just be as simple as passing a different parameter to the formatting function, so it should be fairly trivial.
Whiteboard: [planned-sprint] → [planned-sprint c=3]
Updated•10 years ago
|
Assignee: nobody → david.garciaparedes
Assignee | ||
Comment 4•10 years ago
|
||
Eric, Looking at the code it seems that when the duration is exactly 0, it will say 'Missed' if it is an incoming call or 'Canceled' if it is an outgoing call.
Assignee | ||
Comment 5•10 years ago
|
||
I discussed with :stas in IRC about moving prettyDuration to l10n_date: Quoting him: "semantically I don't see an issue, but I'd like to avoid growing l10n_date too much, b/c it's used in many apps. I wouldn't be opposed to having shared/js/l10n_duration.js if there's a needfor this logic to be used outside of Dialer" Because currently this seems to be only used in dialer and I'm not sure other applications will want to show durations with this specific format, I think we can probably leave it in dialer/utils.
Reporter | ||
Comment 6•10 years ago
|
||
Actually, if you pick up and hang up an incoming call really quick, you will have a 00:00 call shown on call screen, and in Call information, that's how I got that. https://bug1061120.bugzilla.mozilla.org/attachment.cgi?id=8482633 (In reply to David Garcia [:davidg] from comment #4) > Eric, > Looking at the code it seems that when the duration is exactly 0, it will > say 'Missed' if it is an incoming call or 'Canceled' if it is an outgoing > call.
Comment 7•10 years ago
|
||
I'm not sure we can distinguish a non-connected call and a call of 0 seconds. I don't think it really matters in real world usage though. And if it does, it should be handled in another bug.
Assignee | ||
Comment 8•10 years ago
|
||
Attachment #8483472 -
Flags: review?(drs+bugzilla)
Assignee | ||
Comment 9•10 years ago
|
||
Eric, You are right. The call duration is in miliseconds, so it can be greater than 0 but less than a second. In that case im just showing '0 s'. I'm following your suggestion of using this format '1hr 12min 30s' when it is more than an hour.
Comment 10•10 years ago
|
||
Comment on attachment 8483472 [details] [review] github PR See my comments on the PR.
Attachment #8483472 -
Flags: review?(drs+bugzilla) → review-
Assignee | ||
Updated•10 years ago
|
Attachment #8483472 -
Flags: review- → review?(drs+bugzilla)
Assignee | ||
Comment 12•10 years ago
|
||
Fixed comments
Comment 13•10 years ago
|
||
Comment on attachment 8483472 [details] [review] github PR I'd like to see another version, but I think we'll be good to go after that. See comments on PR.
Attachment #8483472 -
Flags: review?(drs+bugzilla) → review-
Assignee | ||
Updated•10 years ago
|
Attachment #8483472 -
Flags: review- → review?(drs+bugzilla)
Assignee | ||
Updated•10 years ago
|
Status: NEW → ASSIGNED
Comment 14•10 years ago
|
||
Comment on attachment 8483472 [details] [review] github PR I left a few small comments on the PR, but this otherwise looks good.
Attachment #8483472 -
Flags: review?(drs+bugzilla) → review+
Comment 15•10 years ago
|
||
https://github.com/mozilla-b2g/gaia/commit/b2bc7392fd1a1407ba3fc774fd84e185a5a09365
Comment 16•10 years ago
|
||
Note that due to recent policy changes, all patches must have approval for uplift regardless of blocking status. Please request Gaia v2.1 approval on this patch when you get a chance. Sorry for the inconvenience :(
Flags: needinfo?(david.garciaparedes)
Comment 17•10 years ago
|
||
+callDurationTextSeconds={{ s }} s +callDurationTextMinutes={{ m }} min {{ s }} s +callDurationTextHours={{ h }} hr {{ m }} min {{ s }} s Is this text coming from a spec? Because the mix of abbreviations looks awkward to me: hr/min/s. I'd expect h/m/s, or hr/min/sec. The first one would be probably more consistent with what we already use around Gaia.
Assignee | ||
Comment 18•10 years ago
|
||
Francesco, In the spec [1] page 5 it says "we adopt “min” and “s” as the abbreviation of “minutes” and “seconds” to display the call length." [1] https://bug877971.bugzilla.mozilla.org/attachment.cgi?id=8463237
Assignee | ||
Comment 19•10 years ago
|
||
Comment on attachment 8483472 [details] [review] github PR [Approval Request Comment] [Bug caused by] (feature/regressing bug #): 877971 [User impact] if declined: Call duration in call info page will be displayed with this format "05:10" instead of this format "5 min 10 s" [Testing completed]: Added unit tests in apps/sharedtest/test/unit/dialer/utils_test.js [Risk to taking this patch] (and alternatives if risky): The function changed it is also used in call screen for displaying call duration, but it has been tested that it works the same as before. [String changes made]: Added 3 new string in apps/sharedtest/test/unit/dialer/utils_test.js +callDurationTextSeconds={{ s }} s +callDurationTextMinutes={{ m }} min {{ s }} s +callDurationTextHours={{ h }} hr {{ m }} min {{ s }} s Requesting approval for 2.1
Attachment #8483472 -
Flags: approval-gaia-v2.1?(release-mgmt)
Flags: needinfo?(david.garciaparedes)
Comment 20•10 years ago
|
||
Setting a ni? for Stephany. Have a look at comment 17 and comment 18 in particular. Thanks.
Flags: needinfo?(swilkes)
Comment 21•10 years ago
|
||
Matej, is this flag for patch approval? If so, we need to be flagged on ui-review? for the actual patch so I know which one to pull and check on the device before this can be approved for uplift. Won't the "min" and "s" have to be translated, and thus count as new strings and be checked for translation issues? It does look like new strings were added per comment #19, but I'm just wondering if this format actually localizes well or is what users expect in our markets. Regardless, has that happened or is this now in late-l10n territory?
Flags: needinfo?(swilkes)
Comment 22•10 years ago
|
||
(In reply to Stephany Wilkes from comment #21) > Matej, is this flag for patch approval? If so, we need to be flagged on > ui-review? for the actual patch so I know which one to pull and check on the > device before this can be approved for uplift. > > Won't the "min" and "s" have to be translated, and thus count as new strings > and be checked for translation issues? It does look like new strings were > added per comment #19, but I'm just wondering if this format actually > localizes well or is what users expect in our markets. Regardless, has that > happened or is this now in late-l10n territory? I have to admit I know very little about this bug. I was just added yesterday, but didn't see any questions specifically for me. I just wanted to make sure you added it to the audit. Sorry for the confusion.
Comment 23•10 years ago
|
||
(In reply to Stephany Wilkes from comment #21) > Matej, is this flag for patch approval? If so, we need to be flagged on > ui-review? for the actual patch so I know which one to pull and check on the > device before this can be approved for uplift. My question wasn't about uplift or late-l10n, more about the mix of abbreviations. I don't think we ever used 'hr' in Gaia for example.
Comment 24•10 years ago
|
||
Flagging Joe. Joe, do you know if this is a) a standard time display and really b) whether or not a standard display is actually required? Just double checking the requirements on time display before it lands.
Comment 25•10 years ago
|
||
Stephany: looks like you forgot to needinfo the Joe you're talking about :)
Flags: needinfo?(swilkes)
Comment 26•10 years ago
|
||
Ha - thanks for the catch, Rik! :)
Flags: needinfo?(swilkes) → needinfo?(jcheng)
Comment 27•10 years ago
|
||
Yes, 1. If users pick up the call and hang up immediately so the call is exactly 0 minute, we shown "0 min 0 s" 2. I'd say adopt "hr" as "hour", but I think we need to ask some localization folks to see if this is a standard format. Thanks!
Flags: needinfo?(cawang)
Comment 28•10 years ago
|
||
After an IRC discussion, Carrie is going to ask on the world-ready mailing list what the standardized format should be. We may have to back this out or do a followup for it if the format is wrong.
Flags: needinfo?(cawang)
Comment 29•10 years ago
|
||
not sure if this is a requirement coming from any partner but i don't think so (ni? WDM) my 2 cents hour, min, sec probably varies per locale this means we may need to keep a table of the standard format display per locale (if we don't already have such table) and run into risks of an overly crowded line for some locales. (not sure if in some locale min/sec goes before the numbers like min 00 sec 05?, just my speculation). Perhaps a locale independent solution can be considered here
Flags: needinfo?(jcheng) → needinfo?(wmathanaraj)
Comment 30•10 years ago
|
||
I'm not sure this is material for world-ready: since the string is localizable, people will localize 'hr' as they consider fit. Having said that. 1. If you use 'h' or 'hr', we can live without proper plural forms. If you use "hour", we need a proper plural form (1 hour, 2 hours, etc.) and a lot of space. 2. (my first and main question) en-US is not being consistent We use 'h' here (I think for all notifications in the tray) https://hg.mozilla.org/gaia-l10n/en-US/file/default/shared/date/date.properties#l172 And, if you're curious, you can also see how that string is currently localized http://transvision.mozfr.org/string/?entity=shared/date/date.properties:hours-ago-short%5Bother%5D&repo=gaia
Assignee | ||
Comment 31•10 years ago
|
||
Being this a late-l10n bug, tomorrow is the last day for a possible land in 2.1. Please let me know if you have any conclusion so I can prepare a patch otherwise it can not be fixed in 2.1 version
Assignee | ||
Comment 32•10 years ago
|
||
Attachment #8487950 -
Flags: review?(drs+bugzilla)
Comment 33•10 years ago
|
||
I've decided to take Francesco's suggestion since he's the only one who has shown any degree of certainty. attachment 8487950 [details] [review] moves us from the "hr min s" format to "h m s". Once it lands, we're going to request uplift of both patches. If we want to change this afterwards, we can, just not within the 2.1 timeframe.
Updated•10 years ago
|
Attachment #8487950 -
Flags: review?(drs+bugzilla) → review+
Comment 34•10 years ago
|
||
https://github.com/mozilla-b2g/gaia/commit/8cbf8501495d871e0a2783b41ddbba2c1ec46993
Comment 35•10 years ago
|
||
per comment 30, I think we should just adopt "h m s" for now. If there has any concern, we can change it later. Thanks!
Flags: needinfo?(cawang)
Assignee | ||
Comment 36•10 years ago
|
||
Comment on attachment 8487950 [details] [review] Follow-up patch changing strings [Approval Request Comment] [Bug caused by] (feature/regressing bug #): 877971 [User impact] if declined: Call duration format will be hr/min/s instead of h/m/s [Testing completed]: yes [Risk to taking this patch] (and alternatives if risky): [String changes made]: yes This needs to be landed AFTER the previous patch in this bug.
Attachment #8487950 -
Flags: approval-gaia-v2.1?(release-mgmt)
Comment 37•10 years ago
|
||
Comment on attachment 8483472 [details] [review] github PR Requesting QA verification on testing around call screen duration displaying as expected on landing.
Attachment #8483472 -
Flags: approval-gaia-v2.1?(release-mgmt) → approval-gaia-v2.1+
Updated•10 years ago
|
Attachment #8487950 -
Flags: approval-gaia-v2.1?(release-mgmt) → approval-gaia-v2.1+
Comment 38•10 years ago
|
||
Needs rebasing for v2.1 uplift.
Flags: needinfo?(david.garciaparedes)
Keywords: branch-patch-needed
Assignee | ||
Comment 39•10 years ago
|
||
Flags: needinfo?(david.garciaparedes)
Updated•10 years ago
|
Keywords: branch-patch-needed
Comment 40•10 years ago
|
||
v2.1: https://github.com/mozilla-b2g/gaia/commit/59e5c2467b7b8219ed194a0d0a94c6ed59af95be
Comment 41•10 years ago
|
||
This issue is verified fixed on Flame 2.2 Master KK (319mb) (Full Flash) and Flame 2.1 KK (319mb) (Full Flash) Environmental Variables: Device: Flame 2.2 Master KK (319mb) (Full Flash) BuildID: 20141017040208 Gaia: abef62c0623e5504a97b4fd411e879a67b285b52 Gecko: ae1dfa192faf Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf Version: 36.0a1 Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0 -------------------------------------------------------------------------------------- Environmental Variables: Device: Flame 2.1 KK (319mb) (Full Flash) BuildID: 20141017001201 Gaia: 1ea74943cfe525c76a074ca1d7de8e51a70f6b98 Gecko: 2befa902ff5c Gonk: 05aa7b98d3f891b334031dc710d48d0d6b82ec1d Version: 34.0 Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 Result: Time duration format SHOWS 'min' or 's' in 'Call Information'
Status: RESOLVED → VERIFIED
QA Whiteboard: [COM=Gaia::Dialer] → [COM=Gaia::Dialer][QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Keywords: verifyme
Comment 42•10 years ago
|
||
Fred, can you recheck this? It should be showing h m s. Please confirm.
QA Whiteboard: [COM=Gaia::Dialer][QAnalyst-Triage?] → [COM=Gaia::Dialer][QAnalyst-Triage-]
Flags: needinfo?(ktucker) → needinfo?(fmuyumba)
Comment 43•10 years ago
|
||
Confirming that issue is verified fixed in Flame 2.2, 2.1 (Full Flash, Nightly). Actual Results: Elapsed time is correctly logged in Call Information screen. Hours, seconds, and minutes are shown correctly. Device: Flame Master Build ID: 20141020040204 Gaia: dc496d04907dd314f9736ff78bab3bd27156f79a Gecko: ca6b46cbca3b Version: 36.0a1 (Master) Firmware Version: v180 User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0 Device: Flame 2.1 Build ID: 20141020001201 Gaia: 2904ab80816896f569e2d73958427fb82aebaea5 Gecko: 12dc9b782f2a Version: 34.0 (2.1) Firmware Version: v180 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
QA Whiteboard: [COM=Gaia::Dialer][QAnalyst-Triage-] → [COM=Gaia::Dialer][QAnalyst-Triage?]
Flags: needinfo?(fmuyumba) → needinfo?(ktucker)
Updated•10 years ago
|
Flags: needinfo?(wmathanaraj)
Updated•10 years ago
|
QA Whiteboard: [COM=Gaia::Dialer][QAnalyst-Triage?] → [COM=Gaia::Dialer][QAnalyst-Triage+_]
Flags: needinfo?(ktucker)
You need to log in
before you can comment on or make changes to this bug.
Description
•