New issue
Advanced search Search tips

Issue 846723 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Date.prototype.setDate doesn't convert float to integer

Reported by kossnoc...@gmail.com, May 25 2018

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.181 Safari/537.36

Steps to reproduce the problem:
const date = new Date(2018, 1)
date.setDate(11.2)
console.assert(date.getHours() === 0, "The hour should't be changed")

What is the expected behavior?
setDate should round the passed value.

What went wrong?
The passed value wasn't rounded so it affected the time.

Did this work before? N/A 

Chrome version: 66.0.3359.181  Channel: stable
OS Version: OS X 10.13.3
Flash Version:
 
Image 2018-05-25 at 6.22.38 PM.png
83.0 KB View Download
Image 2018-05-25 at 6.23.54 PM.png
139 KB View Download
Here is the issue in date-fns that made us discover the problem: https://github.com/date-fns/date-fns/issues/746

Comment 2 by woxxom@gmail.com, May 25 2018

Bisected to 9e217ee4900a6b598967039e936af12dc3c4d06b
"[builtins] Refactor the remaining Date builtins"
Landed in 49.0.2620.0 via r368886
Labels: Needs-Triage-M66
Labels: Triaged-ET M-69 Target-69 FoundIn-69 OS-Linux OS-Windows
Status: Untriaged (was: Unconfirmed)
Able to reproduce the issue on Mac 10.13.3, Win-10 and Ubuntu 17.10 using chrome reported version #66.0.3359.181 and latest canary #69.0.3442.0.
This is a non-regression issue as it is observed from M60 old builds. 

Hence, marking it as untriaged to get more inputs from dev team.

Thanks...!!

Comment 5 by e...@chromium.org, May 29 2018

Components: -Blink Blink>JavaScript>API
I'd like to fix this.
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/10cfe818bd3e6fff08a4e5d17be11a94021b2d9d

commit 10cfe818bd3e6fff08a4e5d17be11a94021b2d9d
Author: Amos Lim <eui-sang.lim@samsung.com>
Date: Tue Jun 05 09:27:39 2018

[builtins] Convert double to integer in Date.prototype.setDate

date in Makeday should be converted to integer.
https://www.ecma-international.org/ecma-262/5.1/#sec-15.9.1.12

Bug:  v8:7475 , chromium:846723 
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Change-Id: I3aa725e7ce1822345502284aec919695c4ca084d
Reviewed-on: https://chromium-review.googlesource.com/1080110
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53516}
[modify] https://crrev.com/10cfe818bd3e6fff08a4e5d17be11a94021b2d9d/AUTHORS
[modify] https://crrev.com/10cfe818bd3e6fff08a4e5d17be11a94021b2d9d/src/builtins/builtins-date.cc
[modify] https://crrev.com/10cfe818bd3e6fff08a4e5d17be11a94021b2d9d/test/mjsunit/date.js
[modify] https://crrev.com/10cfe818bd3e6fff08a4e5d17be11a94021b2d9d/test/test262/test262.status

Cc: eui-sang.lim@samsung.com
Owner: jgruber@chromium.org
Status: Fixed (was: Untriaged)
Labels: Merge-Request-68
Project Member

Comment 10 by sheriffbot@chromium.org, Jun 6 2018

Labels: -Merge-Request-68 Hotlist-Merge-Approved Merge-Approved-68
Your change meets the bar and is auto-approved for M68. Please go ahead and merge the CL to branch 3440 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: TE-Verified-69.0.3451.0 TE-Verified-M69
Able to reproduce the issue on Mac 10.13.3 using chrome build without fix.

Verified the fix on Mac 10.13.3, Win-10 and Ubuntu 17.10 using Chrome version #69.0.3451.0 as per the issue id: v8:7475  in comment #7.
Attaching screen shot for reference.
Observed that setDate rounded the passed value.
Hence, the fix is working as expected. 
Adding the verified labels.

Thanks...!!
846723.png
428 KB View Download
Project Member

Comment 12 by bugdroid1@chromium.org, Jun 6 2018

Labels: merge-merged-6.8
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/d95c2e8041c7bd46f947794a00b5b9ccda7f093e

commit d95c2e8041c7bd46f947794a00b5b9ccda7f093e
Author: jgruber <jgruber@chromium.org>
Date: Wed Jun 06 12:20:22 2018

Merged: [builtins] Convert double to integer in Date.prototype.setDate

This is a combined merge of two CLs:

[builtins] Convert double to integer in Date.prototype.setDate
https://crrev.com/c/1080110

[date] Fix double-to-int conversion in MakeDay
https://crrev.com/c/1087063

No-Try: true
No-Presubmit: true
No-Treechecks: true
Bug:  v8:7475 , chromium:846723 
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Change-Id: I3aa725e7ce1822345502284aec919695c4ca084d
Reviewed-on: https://chromium-review.googlesource.com/1080110
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#53516}(cherry picked from commit 10cfe818bd3e6fff08a4e5d17be11a94021b2d9d)
Reviewed-on: https://chromium-review.googlesource.com/1088790
Cr-Commit-Position: refs/branch-heads/6.8@{#15}
Cr-Branched-From: 44d7d7d6b1041b57644400a00cb3fee35f6c51b2-refs/heads/6.8.275@{#1}
Cr-Branched-From: 5754f66f75136dc17b4c63fec84f31dfdb89186e-refs/heads/master@{#53286}
[modify] https://crrev.com/d95c2e8041c7bd46f947794a00b5b9ccda7f093e/AUTHORS
[modify] https://crrev.com/d95c2e8041c7bd46f947794a00b5b9ccda7f093e/src/builtins/builtins-date.cc
[modify] https://crrev.com/d95c2e8041c7bd46f947794a00b5b9ccda7f093e/test/mjsunit/date.js
[add] https://crrev.com/d95c2e8041c7bd46f947794a00b5b9ccda7f093e/test/mjsunit/regress/regress-849663.js
[modify] https://crrev.com/d95c2e8041c7bd46f947794a00b5b9ccda7f093e/test/test262/test262.status

Labels: -Merge-Approved-68
And merged to 68. Thanks all!
👏

Sign in to add a comment