Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(479)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 394903004: document.lastModified should consider user's local time zone (Closed)

Created:
6 years, 5 months ago by kangil_
Modified:
6 years, 4 months ago
Reviewers:
tkent, haraken
CC:
blink-reviews, blink-reviews-dom_chromium.org, dglazkov+blink, sof, eae+blinkwatch, rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

document.lastModified should consider user's local time zone SPEC: http://www.whatwg.org/specs/web-apps/current-work/#dom-document-lastmodified Failed test cases will be passed with this CL in http://w3c-test.org/html/dom/documents/resource-metadata-management/document-lastModified-01.html Behavior in other browsers. *)FF: PASS *)IE: PASS In addition, this CL consolidates a function that provides local time on given argument. Subsequently, calculation method for UTC and DST are now same with Date. TEST=LayoutTests/http/tests/misc/last-modified-parsing.html Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=179277

Patch Set 1 #

Patch Set 2 : Fix failed layout tests cases #

Patch Set 3 : Retry #

Patch Set 4 : Remove unnecessary hunk #

Patch Set 5 : Remove an obsolete test case #

Patch Set 6 : Rebase #

Total comments: 14

Patch Set 7 : Take review comment into consideration #

Patch Set 8 : Fix a failed layout tests case #

Total comments: 2

Patch Set 9 : Take review comment into consideration #

Patch Set 10 : Rebase and fix day correctly in test case #

Patch Set 11 : Use preexisted calculation logic #

Patch Set 12 : Debug trybot #

Patch Set 13 : Debug 2 #

Patch Set 14 : Date.UTC #

Patch Set 15 : Use internals to get system time #

Patch Set 16 : Finger crossed #

Total comments: 2

Patch Set 17 : Match dst calculation with Date #

Patch Set 18 : Debug 3 #

Patch Set 19 : Debug 4 #

Patch Set 20 : Rebase and finger crossed 2 #

Patch Set 21 : Fix windows build error #

Total comments: 14

Patch Set 22 : Rebase and take review comments into consideration #

Unified diffs Side-by-side diffs Delta from patch set Stats (+50 lines, -100 lines) Patch
D LayoutTests/http/tests/misc/last-modified-parsing.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +13 lines, -1 line 0 comments Download
D LayoutTests/http/tests/misc/last-modified-parsing-expected.txt View 1 2 3 4 5 6 1 chunk +7 lines, -7 lines 0 comments Download
M LayoutTests/http/tests/misc/no-last-modified.html View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/http/tests/resources/last-modified.php View 1 2 3 4 5 6 7 8 18 19 1 chunk +8 lines, -1 line 0 comments Download
A + LayoutTests/http/tests/resources/no-last-modified.php View 1 2 3 4 5 6 7 0 chunks +-1 lines, --1 lines 0 comments Download
M Source/core/dom/Document.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 3 chunks +3 lines, -2 lines 0 comments Download
M Source/core/html/forms/BaseDateAndTimeInputType.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +1 line, -5 lines 0 comments Download
M Source/core/html/forms/MonthInputType.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +1 line, -7 lines 0 comments Download
M Source/core/html/forms/TimeInputType.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +1 line, -7 lines 0 comments Download
M Source/core/html/shadow/DateTimeFieldElements.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +1 line, -7 lines 0 comments Download
M Source/wtf/DateMath.h View 1 2 3 4 5 6 2 chunks +3 lines, -5 lines 0 comments Download
M Source/wtf/DateMath.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 6 chunks +12 lines, -58 lines 0 comments Download

Messages

Total messages: 19 (0 generated)
kangil_
PTAL
6 years, 5 months ago (2014-07-18 04:45:26 UTC) #1
haraken
I think tkent-san is an expert of this code.
6 years, 5 months ago (2014-07-18 05:24:56 UTC) #2
tkent
https://codereview.chromium.org/394903004/diff/100001/LayoutTests/fast/dom/Document/lastModified-01.html File LayoutTests/fast/dom/Document/lastModified-01.html (right): https://codereview.chromium.org/394903004/diff/100001/LayoutTests/fast/dom/Document/lastModified-01.html#newcode1 LayoutTests/fast/dom/Document/lastModified-01.html:1: <!DOCTYPE html> This test is flaky, and unacceptable. We ...
6 years, 5 months ago (2014-07-18 14:16:56 UTC) #3
tkent
https://codereview.chromium.org/394903004/diff/100001/LayoutTests/fast/dom/Document/lastModified-01.html File LayoutTests/fast/dom/Document/lastModified-01.html (right): https://codereview.chromium.org/394903004/diff/100001/LayoutTests/fast/dom/Document/lastModified-01.html#newcode1 LayoutTests/fast/dom/Document/lastModified-01.html:1: <!DOCTYPE html> On 2014/07/18 14:16:56, tkent wrote: > This ...
6 years, 5 months ago (2014-07-18 14:17:26 UTC) #4
kangil_
PTAL https://codereview.chromium.org/394903004/diff/100001/LayoutTests/fast/dom/Document/lastModified-01.html File LayoutTests/fast/dom/Document/lastModified-01.html (right): https://codereview.chromium.org/394903004/diff/100001/LayoutTests/fast/dom/Document/lastModified-01.html#newcode1 LayoutTests/fast/dom/Document/lastModified-01.html:1: <!DOCTYPE html> On 2014/07/18 14:16:56, tkent wrote: > ...
6 years, 5 months ago (2014-07-21 02:34:42 UTC) #5
kangil_
Seems linux_blink_rel trybot doesn't reflect my change. That test case is passed on my side.
6 years, 5 months ago (2014-07-21 05:18:04 UTC) #6
tkent
The CL looks good. We need to investigate the test failure on the try bot. ...
6 years, 5 months ago (2014-07-22 00:48:41 UTC) #7
kangil_
Debug shows that linux trybot uses +16 hours of DST. So, considering typical DST is ...
6 years, 5 months ago (2014-07-23 06:35:53 UTC) #8
tkent
On 2014/07/23 06:35:53, kangil_ wrote: > Debug shows that linux trybot uses +16 hours of ...
6 years, 5 months ago (2014-07-23 06:51:36 UTC) #9
kangil_
On 2014/07/23 06:51:36, tkent wrote: > Oh, it's funny. However, Blink should work well with ...
6 years, 5 months ago (2014-07-24 02:16:20 UTC) #10
tkent
https://codereview.chromium.org/394903004/diff/300001/LayoutTests/http/tests/misc/last-modified-parsing.html File LayoutTests/http/tests/misc/last-modified-parsing.html (right): https://codereview.chromium.org/394903004/diff/300001/LayoutTests/http/tests/misc/last-modified-parsing.html#newcode12 LayoutTests/http/tests/misc/last-modified-parsing.html:12: date = new Date(internals.convertToLocalTime(ms)); Did you investigate why local ...
6 years, 5 months ago (2014-07-24 06:20:05 UTC) #11
kangil_
https://codereview.chromium.org/394903004/diff/300001/LayoutTests/http/tests/misc/last-modified-parsing.html File LayoutTests/http/tests/misc/last-modified-parsing.html (right): https://codereview.chromium.org/394903004/diff/300001/LayoutTests/http/tests/misc/last-modified-parsing.html#newcode12 LayoutTests/http/tests/misc/last-modified-parsing.html:12: date = new Date(internals.convertToLocalTime(ms)); On 2014/07/24 06:20:05, tkent wrote: ...
6 years, 5 months ago (2014-07-24 11:35:28 UTC) #12
kangil_
On 2014/07/24 11:35:28, kangil_ wrote: > After further investigation, I've found that calculating dst offset ...
6 years, 5 months ago (2014-07-25 10:38:35 UTC) #13
kangil_
Trybots look all happy. PTAL!
6 years, 5 months ago (2014-07-25 10:46:36 UTC) #14
tkent
lgtm with nits https://codereview.chromium.org/394903004/diff/400001/LayoutTests/http/tests/misc/last-modified-parsing.html File LayoutTests/http/tests/misc/last-modified-parsing.html (right): https://codereview.chromium.org/394903004/diff/400001/LayoutTests/http/tests/misc/last-modified-parsing.html#newcode9 LayoutTests/http/tests/misc/last-modified-parsing.html:9: var month = ("0" + (date.getMonth() ...
6 years, 4 months ago (2014-07-28 00:40:27 UTC) #15
kangil_
Thx! https://codereview.chromium.org/394903004/diff/400001/LayoutTests/http/tests/misc/last-modified-parsing.html File LayoutTests/http/tests/misc/last-modified-parsing.html (right): https://codereview.chromium.org/394903004/diff/400001/LayoutTests/http/tests/misc/last-modified-parsing.html#newcode9 LayoutTests/http/tests/misc/last-modified-parsing.html:9: var month = ("0" + (date.getMonth() + 1)).slice(-2); ...
6 years, 4 months ago (2014-07-31 00:45:46 UTC) #16
kangil_
The CQ bit was checked by kangil.han@samsung.com
6 years, 4 months ago (2014-07-31 00:45:58 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kangil.han@samsung.com/394903004/420001
6 years, 4 months ago (2014-07-31 00:46:21 UTC) #18
commit-bot: I haz the power
6 years, 4 months ago (2014-07-31 01:55:22 UTC) #19
Message was sent while issue was closed. 
Change committed as 179277

Powered by Google App Engine
This is Rietveld 408576698