[Webkit-unassigned] [Bug 130967] erroneous date calculations on march 30, 2014 (CET/CEST time zone) the day on which daylight savings time changes

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=130967

--- Comment #58 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 240259
  --> https://bugs.webkit.org/attachment.cgi?id=240259
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=240259&action=review

In light of your comment, I'd like to request one more change (and threw in a few more nits while I was at it).  Can you make these changes please?  Thanks.

> Source/JavaScriptCore/runtime/DatePrototype.cpp:944
> +    const WTF::TimeType inputTimeType = WTF::LocalTime;
> +    return setNewValueFromTimeArgs(exec, 1, inputTimeType);

nit: This can be simplified to:
    return setNewValueFromTimeArgs(exec, 1, WTF::LocalTime);

I don't think there's that much to be gained by using 2 statements here.  Can you also apply this to the other functions below which is doing the same thing?

>> Source/JavaScriptCore/runtime/JSDateMath.cpp:200
>> +        result -= localTimeOffset(vm, result, inputTimeType).offset; // result is in local time milliseconds.
> 
> Previously, you said that "The result value can be local time and UTC time according to InputTimeType.”  To clarify, what do you mean when in this added comment when you said that the "result is in local time milliseconds”?  Did you mean that the previous result was in local time, and that you’re converting it to UTC time now?  Or did you mean the previous result is in UTC time and you’re converting it to local time?

(In reply to comment #57)
> When the result passed in localTimeOffset(vm, result, inputTimeType), it is in the local time. After result value compensated by it's result by "result -= localTimeOffset(vm, result, inputTimeType).offset;", it is turned into UTC time.

It looks like the result is always expected to be in UTC time.  In that case, let's rename "result" to "utcTimeResult" and remove the comment.  I think that that should make it clear that we had a local time input here and we want a UTC time result.

> Source/WTF/wtf/DateMath.cpp:1105
> +        offset = calculateLocalTimeOffset(ms, LocalTime).offset / msPerMinute; // ms value is local time milliseconds.

nit: "is local" ==> "is in local".

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141022/39bb7716/attachment-0002.html>