27020 2009-07-06 20:03:13 -0700 msToGregorianDateTime ignores outputIsUTC param when filling in utcOffset 2009-07-15 09:59:00 -0700 1 1 1 Unclassified WebKit Web Template Framework 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 23154 0 joenotcharles webkit-unassigned eric manyoso yong.li.webkit oldest_to_newest 130113 0 joenotcharles 2009-07-06 20:03:13 -0700 msToGregorianDateTime has an outputIsUTC parameter - if false, the output should be in localtime. If the parameter is true, the result GregorianDateTime structure is filled with a time in UTC, but the utcOffset field is still set to the local timezone offset as if it were filled with a local time. I believe it should always be 0 if outputIsUTC is true. This happens to work in most cases because utcOffset is never used - it happens that everywhere msToGregorianDateTime is called with outputIsUTC true, it's in a code path which doesn't look at utcOffset. I don't think we should be depending on this, though. 130126 1 32358 joenotcharles 2009-07-06 20:58:38 -0700 Created attachment 32358 patch to DateMath.cpp 130137 2 eric 2009-07-07 00:06:20 -0700 So this code path is never used? 130207 3 joenotcharles 2009-07-07 08:03:03 -0700 The most common idiom boils down to: msToGregorianDateTime(millisecs, outputIsUTC, t); if (outputIsUTC) // do something with t.month, t.year, etc. but not t.utcOffset else // do something with t.month, t.year, etc. adjusting for t.utcOffset So the code path in this patch is followed, but in the case where utcOffset is wrong it isn't used anyway. I believe all calls to msToGregorianDateTime work like this, but some of them are pretty complex so I can't be 100% sure. We have this patch in our WinCE branch because at one point Google Calendar didn't work for us without it. I thought that it worked now, but I was submitting it anyway because it seemed like the correct thing to do (and I figured if not we would find that out during the code review.) However, Yong thinks Google Calendar still doesn't work for us without this patch. I'm trying to find a test case now. 131558 4 manyoso 2009-07-14 07:53:25 -0700 Joe, can you update this bug with latest info you have about the testcase? 131675 5 joenotcharles 2009-07-14 14:32:46 -0700 I haven't been able to find any broken behaviour that this patch fixes. However, I also haven't been able to find anything that it breaks, so I think it should go in because it makes the code more resistant to future breakage. 131888 6 32358 staikos 2009-07-15 08:11:36 -0700 Comment on attachment 32358 patch to DateMath.cpp absence of comments otherwise and it looks both correct and reasonable to me, so r+ 131929 7 manyoso 2009-07-15 09:59:00 -0700 Landed with r45918. 32358 2009-07-06 20:58:38 -0700 2009-07-15 08:11:36 -0700 patch to DateMath.cpp bug27020.patch text/plain 1444 joenotcharles ZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZyBiL0phdmFTY3JpcHRDb3JlL0No YW5nZUxvZwppbmRleCAzMzg5MjgxLi40MTMyMGVjIDEwMDY0NAotLS0gYS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMg QEAKKzIwMDktMDctMDYgIFlvbmcgTGkgIDx5b25nLmxpQHRvcmNobW9iaWxlLmNvbT4KKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjcwMjAKKyAgICAgICAgbXNUb0dyZWdvcmlhbkRh dGVUaW1lIHNob3VsZCBzZXQgdXRjT2Zmc2V0IHRvIDAgd2hlbiBvdXRwdXRJc1VUQyBpcyBmYWxz ZQorICAgICAgICAKKyAgICAgICAgKiB3dGYvRGF0ZU1hdGguY3BwOgorICAgICAgICAoV1RGOjpn cmVnb3JpYW5EYXRlVGltZVRvTVMpOgorCiAyMDA5LTA2LTE4ICBZb25nIExpICA8eW9uZy5saUB0 b3JjaG1vYmlsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCmRp ZmYgLS1naXQgYS9KYXZhU2NyaXB0Q29yZS93dGYvRGF0ZU1hdGguY3BwIGIvSmF2YVNjcmlwdENv cmUvd3RmL0RhdGVNYXRoLmNwcAppbmRleCA0N2M5ZDQ0Li4xZDQ4MGVlIDEwMDY0NAotLS0gYS9K YXZhU2NyaXB0Q29yZS93dGYvRGF0ZU1hdGguY3BwCisrKyBiL0phdmFTY3JpcHRDb3JlL3d0Zi9E YXRlTWF0aC5jcHAKQEAgLTIsNiArMiw3IEBACiAgKiBDb3B5cmlnaHQgKEMpIDE5OTktMjAwMCBI YXJyaSBQb3J0ZW4gKHBvcnRlbkBrZGUub3JnKQogICogQ29weXJpZ2h0IChDKSAyMDA2LCAyMDA3 IEFwcGxlIEluYy4gQWxsIHJpZ2h0cyByZXNlcnZlZC4KICAqIENvcHlyaWdodCAoQykgMjAwOSBH b29nbGUgSW5jLiBBbGwgcmlnaHRzIHJlc2VydmVkLgorICogQ29weXJpZ2h0IChDKSAyMDA3LTIw MDkgVG9yY2ggTW9iaWxlLCBJbmMuCiAgKgogICogVGhlIE9yaWdpbmFsIENvZGUgaXMgTW96aWxs YSBDb21tdW5pY2F0b3IgY2xpZW50IGNvZGUsIHJlbGVhc2VkCiAgKiBNYXJjaCAzMSwgMTk5OC4K QEAgLTUxMiw3ICs1MTMsNyBAQCB2b2lkIG1zVG9HcmVnb3JpYW5EYXRlVGltZShkb3VibGUgbXMs IGJvb2wgb3V0cHV0SXNVVEMsIEdyZWdvcmlhbkRhdGVUaW1lJiB0bSkKICAgICB0bS55ZWFyICAg ICA9ICB5ZWFyIC0gMTkwMDsKICAgICB0bS5pc0RTVCAgICA9ICBkc3RPZmYgIT0gMC4wOwogCi0g ICAgdG0udXRjT2Zmc2V0ID0gc3RhdGljX2Nhc3Q8bG9uZz4oKGRzdE9mZiArIHV0Y09mZikgLyBt c1BlclNlY29uZCk7CisgICAgdG0udXRjT2Zmc2V0ID0gb3V0cHV0SXNVVEMgPyAwIDogc3RhdGlj X2Nhc3Q8bG9uZz4oKGRzdE9mZiArIHV0Y09mZikgLyBtc1BlclNlY29uZCk7CiAgICAgdG0udGlt ZVpvbmUgPSBOVUxMOwogfQogCg==