Closed Bug 1139167 Opened 5 years ago Closed 5 years ago
Some birthdays are off by one day in Thunderbird's addressbook
1.71 KB, application/x-shellscript
2.51 KB, patch
|Details | Diff | Splinter Review|
2.61 KB, patch
|Details | Diff | Splinter Review|
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:36.0) Gecko/20100101 Firefox/36.0 Build ID: 20150224222119 Steps to reproduce: 1) Create a new contact in Thunderbird's addressbook. 2) Set birthday to 1956-08-01. 3) Select the created contact so that its contents are displayed in the bottom pane. The system timezone seems relevant here- The problem definitely occurs with Europe/Berlin, but I expect other timezones to be affected, too. Actual results: The bottom pane shows 1956-07-31 (or the localized version of it) Expected results: The bottom pane should show 1956-08-01 (or the localized version of it)
Christian, it would be really great if you could fix this. Can you fix the .toLocaleDateString() function itself?
Assignee: nobody → christian
Depends on: 1122571
As promised, I have written a test script in bash/nodejs which iterates over all timezones and all days in the years between 1900 and 2099 and compares the input date with the output of the current Thunderbird logic and my proposed changed Thunderbird logic. The results confirm that the output date matches the input date with the proposed logic, while the current logic fails this test. Please note that the test uses .toLocaleString() and not .toLocaleDateString() which Thunderbird uses. The reason for this change is that this allows to catch miscalculations even if the date is rendered correctly, but the hour isn't. I do realize that nodejs is v8 and not Gecko, but as explained above I expect this to be implementation-independent. $ bash mozbug-1139167-test.sh | grep -P 'TZ|new=false' # only returns a debug line for each of the 417 timezones listed on my system Attached are patches against trunk and Thunderbird 31 (I'm running arch and it's the stable version there). A quick check in Thunderbird 31 on my system confirms that the patch is working as expected. It would be great if someone could give feedback on this patch and/or could integrate it into the official sources.
You just need to pick a reviewer now. https://wiki.mozilla.org/Modules/MailNews_Core suggests the likes of standard8 and Neil Rashbrook (firstname.lastname@example.org) but there are others.
Status: NEW → ASSIGNED
Product: Thunderbird → MailNews Core
Comment on attachment 8575128 [details] [diff] [review] Patch against comm-central trunk Updated both patches to avoid dropping the fix for bug #522642. Mark, I'm choosing you as you are listed for both addressbook and i18n. Could you review/commit the patch? Thanks in advance.
Attachment #8575128 - Flags: review?(standard8)
Comment on attachment 8575128 [details] [diff] [review] Patch against comm-central trunk This doesn't need specific i18n review, so passing it across to Mike.
Attachment #8575128 - Flags: review?(standard8) → review?(mconley)
Does this also cover the problem that if you enter a birthday of 29/2/2000 (29th Feb), it shows correctly but as soon as you reopen editing the card, it gets modified to 1/3/2000 (3rd Mar). Or is that actually a problem?
(In reply to :aceman from comment #14) > Does this also cover the problem that if you enter a birthday of 29/2/2000 > (29th Feb), it shows correctly but as soon as you reopen editing the card, > it gets modified to 1/3/2000 (3rd Mar). > Or is that actually a problem? No, sadly not, but it was not the intention of the patch to fix it, I was not even aware of this problem. I'll be happy to try my luck on this one as well, but as this is a different issue in a different dialogue I suggest that this is handled in a seperate bug report. Btw, if someone has any input on why we see so many duplicate bug reports for this issue (#1139167) just now (2015), that would be great. Maybe just a coincidence, but I found it surprising.
Why (in Thunderbird) use such a mechanism for a birthday, a simple past event? Especially since Tb allows entering only the year, or only the day of year, it would be better to record only a threesome, not a 'time_t' or broader equivalent. Otherwise users would be required to enter the longitude or zone of the place where the celebree was born, and the hour of birth. Such things survive calendar-change: George Washington was born on February 11, but, because during his lifetime England & her colonies switched to the Gregorian calendar, its anniversary is celebrated on February 22. In the event of a further calendar reform, the notion the eleven month s eleventh day s eleventh hour, in reference to the western-front armistice of WWI, will not be translated, although its anniversary surely will be.
Mike, are you going to be able to look at this soon?
Comment on attachment 8575128 [details] [diff] [review] Patch against comm-central trunk This looks OK to me by inspection, but somebody needs to review the suite bits.
Attachment #8575128 - Flags: review?(mconley) → review+
Comment on attachment 8575128 [details] [diff] [review] Patch against comm-central trunk Alright. Unfortunately I'm unsure how to request review feedback from a second reviewer in the bug's flags. Mark, you are listed as primary SeaMonkey Address book owner; I'm re-cc'ing you as you probably already invested some time by initially reading this bug. Would be great if you could have a second look at it regarding SeaMonkey. I would also be fine with dropping the suite bits from the patch for now, but I thought that it makes sure to keep the code bases aligned.
Comment on attachment 8575128 [details] [diff] [review] Patch against comm-central trunk I think listing standard8 as the owner of Seamonkey AB is a longstanding bug. I think IanN will be a more active reviewer for this. You can request review from another user via the additional review field that crops up at the bottom of the flags list after you already have a reviewer on a patch. It is marked as "addl.". When there are no reviewers yet, you can put multiple people into the first review field that is in the flags list.
Attachment #8575128 - Flags: review?(iann_bugzilla)
Comment on attachment 8575129 [details] [diff] [review] Patch against thunderbird-31.5.0 sources Review of attachment 8575129 [details] [diff] [review]: ----------------------------------------------------------------- I am not sure we can take something like this for TB31. But there may be a chance of it getting into TB38.x. Maybe the trunk version applies there too.
aceman, thanks for the bug wrangling / advice. As far as I understand, this is now awaiting SeaMonkey review and decisions about the target branches. Please let me know if there is something I am supposed to do here at this stage.
Comment on attachment 8575128 [details] [diff] [review] Patch against comm-central trunk r=me just a small note, it is best to provide patches with a context of 8 and set showfunc to true, so in your .hgrc have something like: [defaults] diff = -U 8 -p qdiff = -U 8 -p [diff] git = true showfunc = true unified = 8
Attachment #8575128 - Flags: review?(iann_bugzilla) → review+
Comment on attachment 8575128 [details] [diff] [review] Patch against comm-central trunk a=me for SeaMonkey CLOSED TREE
Thank you for debugging the issue and writing the patch. It's been uploaded as https://hg.mozilla.org/comm-central/rev/17a6a5c2d295
Comment on attachment 8575128 [details] [diff] [review] Patch against comm-central trunk [Approval Request Comment] Regression caused by (bug #): ? User impact if declined: Addressbook can show a birthday off by one day Testing completed (on c-c, etc.): Not yet Risk to taking this patch (and alternatives if risky): ?
Comment on attachment 8575128 [details] [diff] [review] Patch against comm-central trunk https://hg.mozilla.org/releases/comm-aurora/rev/aed081047c5e https://hg.mozilla.org/releases/comm-beta/rev/3ca0490d9fc9
You need to log in before you can comment on or make changes to this bug.