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)
Toolkit
XUL Widgets
Tracking
()
RESOLVED
FIXED
mozilla9
Tracking | Status | |
---|---|---|
firefox9 | --- | fixed |
People
(Reporter: pi, Assigned: mconley)
References
Details
(Whiteboard: [qa?])
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
enndeakin
:
review+
christian
:
approval-mozilla-aurora+
christian
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•16 years ago
|
||
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.
Comment 2•16 years ago
|
||
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.
Reporter | ||
Comment 3•16 years ago
|
||
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.
Reporter | ||
Comment 4•16 years ago
|
||
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.
Reporter | ||
Comment 5•16 years ago
|
||
Reporter | ||
Comment 6•16 years ago
|
||
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/
Assignee | ||
Comment 7•13 years ago
|
||
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)
Assignee | ||
Comment 8•13 years ago
|
||
Locally tested, and seems to fix the issue for bug 677884 and I believe addresses bug 534466.
Comment 9•13 years ago
|
||
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.
Assignee | ||
Comment 10•13 years ago
|
||
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)
Updated•13 years ago
|
Attachment #564661 -
Flags: review?(enndeakin) → review+
Assignee | ||
Comment 11•13 years ago
|
||
Ok, this looks good on try (https://tbpl.mozilla.org/?tree=Try&rev=9438fb599e7f), committed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/79dc3f1ea63a
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
Assignee | ||
Comment 12•13 years ago
|
||
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?
Comment 13•13 years ago
|
||
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 :-)
Assignee | ||
Comment 14•13 years ago
|
||
Ed:
Whoops - you're right - sorry about that. :)
-Mike
Comment 15•13 years ago
|
||
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+
Updated•13 years ago
|
Assignee: nobody → mconley
Assignee | ||
Updated•13 years ago
|
status-firefox9:
--- → fixed
Target Milestone: mozilla10 → mozilla9
Comment 16•13 years ago
|
||
Is there a minimized testcase QA can use to verify this fix?
Whiteboard: [qa?]
Assignee | ||
Comment 17•13 years ago
|
||
Anthony:
There are STR for Thunderbird here: https://bugzilla.mozilla.org/show_bug.cgi?id=534466#c0
-Mike
Comment 18•13 years ago
|
||
(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?
Assignee | ||
Comment 19•13 years ago
|
||
(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
Comment 20•13 years ago
|
||
Okay, we will try to verify the fix using the testcase in comment 19.
Whiteboard: [qa?] → [qa+]
Comment 21•13 years ago
|
||
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.
Description
•