Closed Bug 488627 Opened 16 years ago Closed 13 years ago

Datepicker doesn't work with some short date formats.

Categories

(Toolkit :: XUL Widgets, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla9
Tracking Status
firefox9 --- fixed

People

(Reporter: pi, Assigned: mconley)

References

Details

(Whiteboard: [qa?])

Attachments

(1 file, 4 obsolete files)

The birthday datepicker (including my most current widget for Bug 456024) doesn't work with certain date formats as mentioned in Bug 456024 comment 11. So far, any short date format with MMM and MMMM as the month result in Oct or October between the actual month (as a number from 01 - 12) and day in Vista. The original reporter was using XP. I haven't tried to reproduce it in Linux yet. I'll break out the DOM Inspector and take a look soon. At least one problem lies in the _init method when collapsing elements, and _setFieldValue may need to detect the short date month format and use that instead of 01 through 12.
Attached image Shows the problem in datepickers (obsolete) (deleted) —
The screenshot shows four datepickers (noyear/birthday, popup, grid, and normal) when the short date format contains MMM. All but the grid datepicker are incorrect, and datetimepickers are probably affected as well. MMMM behaves the same way, just with the full month name where the abbreviation is in the datepickers. It looks like the datepickers find the month (and preceding space) as a separator because of a regular expression which assumes a numeric month in the short date format (%x): http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/content/widgets/datetimepicker.xml#790 numberFields is: 0 - 04 Oct 2002 1 - 2 - 04 3 - Oct 4 - 200 5 - 6 - 2 7 - Versus MM/DD/YYYY: 0 - 10/04/2002 1 - 2 - 10 3 - / 4 - 04 5 - / 6 - 2002 7 - The month is October because of line 792.
For what it's worth, this doesn't seem reproduceable on OS X (10.5.6). There are four date formats in OS X: short, medium, long and full. Setting short and medium to dd MMM yy and dd MMM yyyy respectively don't trigger the Oct phenomenon.
I'm moving this into toolkit since it is a bug with normal and popup datepickers, too. (In reply to comment #2) > For what it's worth, this doesn't seem reproduceable on OS X (10.5.6). There > are four date formats in OS X: short, medium, long and full. > > Setting short and medium to dd MMM yy and dd MMM yyyy respectively don't > trigger the Oct phenomenon. Interesting, I'm guessing that evaluating this line in the Error Console shows a numeric month even with MMM set? alert(new Date().toLocaleFormat("%x")); I can reproduce the bug in Windows by changing MM to MMM in the short date format and in Linux by changing %m in d_fmt to %b, for example.
Assignee: joshgeenen+bugzilla → nobody
Status: ASSIGNED → NEW
Component: Address Book → XUL Widgets
Product: MailNews Core → Toolkit
QA Contact: address-book → xul.widgets
Summary: Birthday datepicker doesn't work with all short date formats. → Datepicker doesn't work with some short date formats.
Attached patch A patch that should work with more date formats (obsolete) (deleted) — Splinter Review
This patch (a work in progress) fixes the bug for me with most date formats but still only uses numeric dates regardless of the system format. It replaces the regex with several checks for different formats for the year, month, and date. The separators are hardcoded at the moment. I'll take the bug and create a formal patch for review if this patch is a solution worth pursuing.
Attached image The datepickers after the patch was applied (obsolete) (deleted) —
Here are most of the unique d_fmt variables in /usr/share/i18n/locales/ on my machine running Gentoo. I wrote a script to do this so there may be some mistakes. See the link below for details on what each variable means. %A %d %B %Y %A %d %b %A %d %b %Y %A %d,%B,%Y %A, %B %d, %Y %A, %d %B, %Y %A, %d t %B %d %A %Y %Y-%b-%d %Y-%m-%d %Y.%m.%d %Y.%m.%d. %a %e.%b %Y %d %b %Y %d %b, %Y %d-%m-%Y %d-%m-%y %d. %b %Y %d. %m. %Y %d. %m. %y %d.%m.%Y %d.%m.%Y. %d.%m.%y %d/%d/%Y %d/%m-%Y %d/%m/%Y %d/%m/%y %e-%m-%Y %e.%m.%Y %m/%d/%Y %m/%d/%y http://www.cplusplus.com/reference/clibrary/ctime/strftime/
Blocks: 677884
Blocks: 534466
Attached patch Patch v1 (obsolete) (deleted) — Splinter Review
Here's my first run at a patch. If we cannot figure out the ordering of the month, day and years from the system locale, we default to YYYY/MM/DD. All feedback welcome! -Mike
Attachment #373249 - Attachment is obsolete: true
Attachment #373586 - Attachment is obsolete: true
Attachment #373587 - Attachment is obsolete: true
Attachment #564651 - Flags: review?(enndeakin)
Locally tested, and seems to fix the issue for bug 677884 and I believe addresses bug 534466.
Comment on attachment 564651 [details] [diff] [review] Patch v1 > var dt = new Date(2002,9,4).toLocaleFormat("%x"); > var numberFields = dt.match(numberOrder); > if (numberFields) { >+ this.yearLeadingZero = (numberFields[yi].length > 1); >+ this.monthLeadingZero = (numberFields[mi].length > 1); >+ this.dateLeadingZero = (numberFields[di].length > 1); yi, mi and di aren't defined yet.
Attached patch Patch v2 (deleted) — Splinter Review
Gah, you're absolutely right - great catch! This time, tested with both a funky locale (with the date time format being Tuesday Oct 4), and my standard locale (11-10-2011).
Attachment #564651 - Attachment is obsolete: true
Attachment #564661 - Flags: review?(enndeakin)
Attachment #564651 - Flags: review?(enndeakin)
Attachment #564661 - Flags: review?(enndeakin) → review+
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
Comment on attachment 564661 [details] [diff] [review] Patch v2 It's a pretty safe patch on code that hasn't been changed in quite a while (hg log says July). I'm on the Thunderbird team, and we'd love to get this fixed for TB 9 - even 8, if we can muster it.
Attachment #564661 - Flags: approval-mozilla-beta?
Attachment #564661 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/79dc3f1ea63a Was likely just a one off due to muscle memory, but normally bugs are only marked fixed when they merge to mozilla-central, rather than when they land on inbound :-)
Ed: Whoops - you're right - sorry about that. :) -Mike
Comment on attachment 564661 [details] [diff] [review] Patch v2 Approved for aurora. We are denying this for beta, though feel free to keep landing it on beta relbranches if it is that important.
Attachment #564661 - Flags: approval-mozilla-beta?
Attachment #564661 - Flags: approval-mozilla-beta-
Attachment #564661 - Flags: approval-mozilla-aurora?
Attachment #564661 - Flags: approval-mozilla-aurora+
Assignee: nobody → mconley
Target Milestone: mozilla10 → mozilla9
Is there a minimized testcase QA can use to verify this fix?
Whiteboard: [qa?]
Anthony: There are STR for Thunderbird here: https://bugzilla.mozilla.org/show_bug.cgi?id=534466#c0 -Mike
(In reply to Mike Conley (:mconley) from comment #17) > Anthony: > > There are STR for Thunderbird here: > https://bugzilla.mozilla.org/show_bug.cgi?id=534466#c0 > > -Mike Does this only affect Thunderbird?
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #18) > Does this only affect Thunderbird? No, I imagine this would affect anyone using datepicker with certain region settings for date (for example, https://bug534466.bugzilla.mozilla.org/attachment.cgi?id=417314) -Mike
Okay, we will try to verify the fix using the testcase in comment 19.
Whiteboard: [qa?] → [qa+]
Verified on: Mozilla/5.0 (Windows NT 5.1; rv:9.0) Gecko/20100101 Firefox/9.0 Build ID: 20111206234556 STR: 1. Modify regional settings short-date format to an extended format (eg "MMM", "MMMM"). 2. Restart Firefox so that it reads the new date-format. 3. Evaluate "new Date().toLocaleFormat("%x")" in the Error Console. Since today is 12/12/2011 I changed the date to 12/11/2011 to make sure if the bug still reproduced it was visible. The right date was displayed in the Error Console ("12-Nov-2011", "12-November-2011"). Is this a right way to verify this bug? The steps are those referenced in comment 18, except for the last one (there is no address book in Firefox). I am asking this since I tried to reproduce the bug on some older builds using the above STR and the bug never reproduced.
Whiteboard: [qa+] → [qa?]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: