New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 653557 link

Starred by 1 user
Status: Fixed
Owner:
█ rob.b...@samsung.com
Email to this user bounced
Closed: Oct 2016
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Parsing of minimum finite values fails

Reported by rob.b...@samsung.com, Oct 6 2016 
What steps will reproduce the problem?
(1) Start inspector console
(2) $0.tabIndex = "-2147483648"
(3) issue $0.tabIndex

What is the expected output?

"-2147483648"

What do you see instead?

"-1"

Comment 1 by rob.b...@samsung.com, Oct 6 2016

Labels: OS-All
Patch under review:
https://codereview.chromium.org/2363823004/
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 6 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/54a7c25c0044c02449ee9366c9323ca1d2461b29

commit 54a7c25c0044c02449ee9366c9323ca1d2461b29
Author: rob.buis <rob.buis@samsung.com>
Date: Thu Oct 06 18:30:21 2016

Fix parsing of minimum values

When parsing values that are minimum finite values
(for example std::numeric_limits::min<int>), we end up ignoring
the sign when calling charactersToIntStrict. This method will
then use the std::numeric_limits::max values for parsing, and
since abs(max) is smaller than abs(min) it will bail out and
parseHTMLIntegerInternal will return false.

To fix this simply add the minus sign in the string to be
parsed by charactersToIntStrict.

BUG= 653557 

Review-Url: https://codereview.chromium.org/2363823004
Cr-Commit-Position: refs/heads/master@{#423595}

[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-embedded-expected.txt
[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-forms-expected.txt
[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-grouping-expected.txt
[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-metadata-expected.txt
[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-misc-expected.txt
[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-obsolete-expected.txt
[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-sections-expected.txt
[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-tabular-expected.txt
[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-text-expected.txt
[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/Source/core/html/parser/HTMLParserIdioms.cpp

Comment 3 by rob.b...@samsung.com, Oct 6 2016 

Fixed by r423595.

Comment 4 by ajha@chromium.org, Oct 10 2016

Labels: TE-Verified-55.0.2883.6 TE-Verified-M55
Verified the fix on the latest M-55(55.0.2883.6) on Windows-10, Mac OS 10.11.6 and Linux Ubuntu 14.04. This is working as intended hence adding the verified label.
653557.png
18.4 KB View Download

Comment 5 by rob.b...@samsung.com, Oct 17 2016

Status: Fixed (was: Assigned)
Fixed by r423595.
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/54a7c25c0044c02449ee9366c9323ca1d2461b29

commit 54a7c25c0044c02449ee9366c9323ca1d2461b29
Author: rob.buis <rob.buis@samsung.com>
Date: Thu Oct 06 18:30:21 2016

Fix parsing of minimum values

When parsing values that are minimum finite values
(for example std::numeric_limits::min<int>), we end up ignoring
the sign when calling charactersToIntStrict. This method will
then use the std::numeric_limits::max values for parsing, and
since abs(max) is smaller than abs(min) it will bail out and
parseHTMLIntegerInternal will return false.

To fix this simply add the minus sign in the string to be
parsed by charactersToIntStrict.

BUG= 653557 

Review-Url: https://codereview.chromium.org/2363823004
Cr-Commit-Position: refs/heads/master@{#423595}

[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-embedded-expected.txt
[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-forms-expected.txt
[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-grouping-expected.txt
[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-metadata-expected.txt
[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-misc-expected.txt
[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-obsolete-expected.txt
[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-sections-expected.txt
[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-tabular-expected.txt
[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-text-expected.txt
[modify] https://crrev.com/54a7c25c0044c02449ee9366c9323ca1d2461b29/third_party/WebKit/Source/core/html/parser/HTMLParserIdioms.cpp

Comment 7 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840
►

Sign in to add a comment