[webkit-reviews] review denied: [Bug 130967] erroneous date calculations on march 30, 2014 (CET/CEST time zone) the day on which daylight savings time changes : [Attachment 229431] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Apr 16 07:50:00 PDT 2014
Mark Lam <mark.lam at apple.com> has denied Byungseon Shin <xingri at gmail.com>'s
request for review:
Bug 130967: erroneous date calculations on march 30, 2014 (CET/CEST time zone)
the day on which daylight savings time changes
https://bugs.webkit.org/show_bug.cgi?id=130967
Attachment 229431: Patch
https://bugs.webkit.org/attachment.cgi?id=229431&action=review
------- Additional Comments from Mark Lam <mark.lam at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=229431&action=review
> Source/JavaScriptCore/ChangeLog:10
> + String(new Date(Mar 30 2014 01:00:00)) is wrong in CET
> + https://bugs.webkit.org/show_bug.cgi?id=130967
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + According to calculateLocalTimeOffset define,
> + it accepts utcInMilliseconds as an Argument
> + So, we need to calculate UTC instead of localTime milliseconds
What is the issue? I tried evaluating "String(new Date("Mar 30 2014 01:00:00
UTC+0100”))” in Safari, Chrome, and Firefox (note: CET is UTC+1 according to
http://www.timeanddate.com/library/abbreviations/timezones/), and they all show
the same string. You need a test case that demonstrates the issue and will
serve as a regression test in the future. If appropriate, please add the test
case to an existing Date test.
Also, I fail to follow your logic here. What does calculateLocalTimeOffset()
have to do with your change in parseDateFromNullTerminatedCharacters()?
> Source/WTF/wtf/DateMath.cpp:470
> +double getUTCOffset()
> +{
> + return calculateUTCOffset();
> +}
If we really need to export this, then let’s export calculateUTCOffset(). No
need to create another function.
> Source/WTF/wtf/DateMath.h:159
> +using WTF::getUTCOffset;
> using WTF::calculateLocalTimeOffset;
This shows that we should export WTF::calculateTimeOffset (if needed) instead
of creating a WTF::getUTCOffset just to wrap it but adds nothing.
More information about the webkit-reviews
mailing list