Closed Bug 955359 Opened 7 years ago Closed 7 years ago

Use weekdays instead of dates for "last week" group in log viewer

Categories

(Instantbird :: Other, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aleth, Assigned: aleth)

Details

Attachments

(1 file, 1 obsolete file)

*** Original post on bio 1922 at 2013-04-07 11:07:00 UTC ***

Suggested by Mic. (I think for the "two weeks ago" group it's already the case that dates are more useful.)
*** Original post on bio 1922 at 2013-04-07 11:25:17 UTC ***

Does anyone know if 
1) this would conflict with localization?
2) there is an existing method to get the localized weekday from a Date?
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1922 as attmnt 2330 at 2013-04-07 11:34:00 UTC ***

Using nsIScriptableDateFormat, which we already use for the dates, and which should be localized, though there is this in the idl [1]: "The output of this method depends on the operating system and user settings. Even if you pass a locale code as the first parameter, there are no guarantees about which locale and exact format the returned value uses."

[1] https://mxr.mozilla.org/mozilla-central/source/intl/locale/idl/nsIScriptableDateFormat.idl
Attachment #8354096 - Flags: review?(benediktp)
Attachment #8354096 - Flags: feedback?(florian)
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Comment on attachment 8354096 [details] [diff] [review]
Patch

*** Original change on bio 1922 attmnt 2330 at 2013-04-07 12:25:34 UTC ***

While the code looks fine, the result isn't ideal. Instead of the full names of the day, it only showed two letter abbreviations of the name here. It's an OS thing so it might or might not work for others :(

To fix this I'd suggest to use the full day names defined in the date format bundle [1] like you already do for the month names in [2].

[1] http://mxr.mozilla.org/mozilla-central/source/toolkit/locales/en-US/chrome/global/dateFormat.properties#31
[2] http://lxr.instantbird.org/instantbird/source/instantbird/content/viewlog.js#234
Attachment #8354096 - Flags: review?(benediktp) → review-
Comment on attachment 8354096 [details] [diff] [review]
Patch

*** Original change on bio 1922 attmnt 2330 at 2013-04-08 11:35:06 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354096 - Flags: feedback?(florian)
Attached patch PatchSplinter Review
*** Original post on bio 1922 as attmnt 2331 at 2013-04-08 11:37:00 UTC ***

>While the code looks fine, the result isn't ideal. Instead of the full names of
>the day, it only showed two letter abbreviations of the name here. It's an OS
>thing so it might or might not work for others :(
Thanks for this feedback, I was wondering if something like that might happen on other OS.

This patch uses the full day names explicitly as you suggested.
Attachment #8354097 - Flags: review?(benediktp)
Comment on attachment 8354096 [details] [diff] [review]
Patch

*** Original change on bio 1922 attmnt 2330 at 2013-04-08 11:37:22 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354096 - Attachment is obsolete: true
Comment on attachment 8354097 [details] [diff] [review]
Patch

*** Original change on bio 1922 attmnt 2331 at 2013-04-12 21:04:19 UTC ***

Thanks for the patch!
Attachment #8354097 - Flags: review?(benediktp) → review+
Whiteboard: [checkin-needed]
*** Original post on bio 1922 at 2013-04-13 14:32:05 UTC ***

Fixed in http://hg.instantbird.org/instantbird/rev/d270aa841434
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.4
You need to log in before you can comment on or make changes to this bug.