296770 - Default text in "Mark Messages as Read by Date" dialogue does not respect leading zeros in system date format

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Grey Nicholson (they/them)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050531 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050531 Firefox/1.0+

If the system's date format is set to include leading xeros (e.g. 2005-06-05 and
05/06/2005) the default text in the Mark Messages as Read by Date dialogue omits
the leading zeros (e.g. 2005-6-5 and 5/6/2005).

The rest of Thunderbird's interface (that I've noticed) shows the correct date
format.

Reproducible: Always

Steps to Reproduce:
1. Set the system date format to YYYY-MM-DD.
2. Open the "Mark Messages as Read by Date" dialogue.
Actual Results:  
Default text in the "To:" field is "2005-6-5".

Expected Results:  
"2005-06-05"

This applies to both Thunderbird 1.0.x and 1.1a1.

Comment 1

[Gervase Markham [:gerv]]

This is an automated message, with ID "auto-resolve01".

This bug has had no comments for a long time. Statistically, we have found that
bug reports that have not been confirmed by a second user after three months are
highly unlikely to be the source of a fix to the code.

While your input is very important to us, our resources are limited and so we
are asking for your help in focussing our efforts. If you can still reproduce
this problem in the latest version of the product (see below for how to obtain a
copy) or, for feature requests, if it's not present in the latest version and
you still believe we should implement it, please visit the URL of this bug
(given at the top of this mail) and add a comment to that effect, giving more
reproduction information if you have it.

If it is not a problem any longer, you need take no action. If this bug is not
changed in any way in the next two weeks, it will be automatically resolved.
Thank you for your help in this matter.

The latest beta releases can be obtained from:
Firefox:     http://www.mozilla.org/projects/firefox/
Thunderbird: http://www.mozilla.org/products/thunderbird/releases/1.5beta1.html
Seamonkey:   http://www.mozilla.org/projects/seamonkey/

Comment 2

[Gervase Markham [:gerv]]

This bug has been automatically resolved after a period of inactivity (see above
comment). If anyone thinks this is incorrect, they should feel free to reopen it.

Comment 3

[Magnus Melin [:mkmelin]]

-> NEW using
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20051117 Thunderbird/1.5 ID:2005111705

Comment 4

[Magnus Melin [:mkmelin]]

Attachment: proposed fix

This adds the leading zeros as needed, also cleans up a related false warning found when testing the patch.

Comment 5

[[e-mail]]

Comment on attachment 222655 [details] [diff] [review]
proposed fix

Surely this patch forces leading zeros? Perhaps the correct thing to do here is to have a second preference for leading zeros, but if the date format is 0 then you determine it from the date format service.

Also I don't have r+sr powers over this code.

Comment 6

[Magnus Melin [:mkmelin]]

Attachment: proposed fix, v2

Addressing review comments. Use leading zeros only when really wanted. 

(or did you mean you don't have r powers at all for this, neil?)

Comment 7

[[e-mail]]

Comment on attachment 223357 [details] [diff] [review]
proposed fix, v2

>+      // set up a date with leading zeros for testing if they should be used
>+      var testDateString = dateFormatService.FormatDate("", 
>+                             dateFormatService.dateFormatShort, 1999, 12, 1);
>+      var testArrayOfStrings = testDateString.split(gSearchDateSeparator);
I was trying to figure out why you made a second string and array. Then I realised that the date format tests look for the month and day, and would get confused between a day of "01" and a day of "1". One solution is to use == 1 instead of == "1" as that will match both "1" and "01". Another option is to write gSearchDateLeadingZeros = /0/.test(zeroDateString); - this will be true if the string contains a zero and false if not. Or perhaps you can convince me that your existing way has the most merit.

>-      getLocaleShortDateFormat();
>-    else
>+      initLocaleShortDateFormat();
>+    else 
You accidentally left a space at the end of this line.

>+  if(gSearchDateLeadingZeros && month < 10) 
>+    month = "0" + month;
>+  var date = time.getDate();
>+  if(gSearchDateLeadingZeros && date < 10) 
>+    date = "0" + date;
Please put a space between all your if and (s.

Comment 8

[Magnus Melin [:mkmelin]]

(In reply to comment #7)
> realised that the date format tests look for the month and day, and would get
> confused between a day of "01" and a day of "1". One solution is to use == 1
> instead of == "1" as that will match both "1" and "01". Another option is to

My approach was to test just that, whether it's "1" or "01", so that wouldn't be very useful. 
Your regexp solution seems good though (lesser code), I'll make a new patch. 

Comment 9

[Magnus Melin [:mkmelin]]

Attachment: proposed fix, v3

Addressing review comments.

Comment 10

[[e-mail]]

Comment on attachment 223436 [details] [diff] [review]
proposed fix, v3

>+  if( gSearchDateLeadingZeros && month < 10 )
>+  if( gSearchDateLeadingZeros && date < 10 )
>+        if( !header.isRead ) { // don't do anything until really necessary
Nit: these three lines still missing a space between if and (

Comment 11

[Magnus Melin [:mkmelin]]

Attachment: proposed fix, v3 + nits picked

Nits picked, carrying forwards neils r+, seeking sr.

Comment 12

[Magnus Melin [:mkmelin]]

Attachment: proposed fix, v3 (unbitrotted)

The previous patch bitrotted (fix for bug 351864 was included in this one).

The patch has r. David, are you able to sr?

Comment 13

[David :Bienvenu]

Comment on attachment 246172 [details] [diff] [review]
proposed fix, v3 (unbitrotted)

sure, thx for the patch.

Comment 14

[[e-mail]]

Fix checked in.

Comment 15

[Mike Cowperthwaite]

Since the date right now has no leading zeroes, if I try "Mark by date" I can't test this (without resetting my computer's calendar).  But I created a date-based message filter, and the fix applies there as well, in TB 3a1-1123, Win2K.

I guess this was a relatively simple bug -- but Magnus, now that you're familiar with this code, could you take a whack at bug 224109, and maybe 
bug 228868?  :)

Comment 16

[Magnus Melin [:mkmelin]]

We'll see... can't say it's a top priority atm.

Comment 17

[Scott MacGregor]

Comment on attachment 246172 [details] [diff] [review]
proposed fix, v3 (unbitrotted)

I didn't notice this approval until after the l10n string freeze. Sorry Magnus.