The default bug view has changed. See this FAQ.

Datepicker doesn't work with some short date formats.

RESOLVED FIXED in Firefox 9

Status

()

Toolkit
XUL Widgets
RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: pi, Assigned: mconley)

Tracking

(Blocks: 1 bug)

Trunk
mozilla9
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox9 fixed)

Details

(Whiteboard: [qa?])

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

8 years ago
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

8 years ago
Created attachment 373249 [details]
Shows the problem in datepickers

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

8 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

8 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

8 years ago
Created attachment 373586 [details] [diff] [review]
A patch that should work with more date formats

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

8 years ago
Created attachment 373587 [details]
The datepickers after the patch was applied
(Reporter)

Comment 6

8 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/
Blocks: 677884
Blocks: 534466
Created attachment 564651 [details] [diff] [review]
Patch v1

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 9

6 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.
Created attachment 564661 [details] [diff] [review]
Patch v2

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

6 years ago
Attachment #564661 - Flags: review?(enndeakin) → review+
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
Last Resolved: 6 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 15

6 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+
Assignee: nobody → mconley
status-firefox9: --- → fixed
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+]

Comment 21

5 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.