Issue metadata
Sign in to add a comment
|
DevTools elements highlighting doesn't work with non-US locale
Reported by
alshaba...@yandex-team.ru,
Apr 18 2017
|
||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.137 YaBrowser/17.4.1.353 (beta) Yowser/2.5 Safari/537.36 Steps to reproduce the problem: 1. Run chrome from command line with LANG=ru_RU.UTF-8 () 2. Open DevTools Elements panel 3. Hover mouse over DOM elements What is the expected behavior? Elements on the web page get highlighted. What went wrong? Elements are not highlighted. Did this work before? N/A Chrome version: 60.0.3074.0 Channel: n/a OS Version: OS X 10.12.4 Flash Version: Shockwave Flash 25.0 r0 Quick investigation points to std::strtod in third_party/inspector_protocol/lib/Parser_cpp.template which parses double with respect to locale.
,
Apr 18 2017
,
Apr 27 2017
Issue 715313 has been merged into this issue.
,
Apr 27 2017
Can we get this fixed before M60 hits stable.
,
Apr 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/aed22ad12f66fdf95ea30fe857253f4d055fc8fb commit aed22ad12f66fdf95ea30fe857253f4d055fc8fb Author: kozyatinskiy <kozyatinskiy@chromium.org> Date: Fri Apr 28 01:43:03 2017 Roll third_party/inspector_protocol to efefa86c3183d307f0a0e53bf568fe57c5b58849 This roll includes: - [inspector_protocol] added StringUtil::toDouble method as requirement [1] [1] https://codereview.chromium.org/2843223005/ BUG= chromium:712610 R=dgozman@chromium.org Review-Url: https://codereview.chromium.org/2846673005 Cr-Commit-Position: refs/heads/master@{#44954} [modify] https://crrev.com/aed22ad12f66fdf95ea30fe857253f4d055fc8fb/src/inspector/DEPS [modify] https://crrev.com/aed22ad12f66fdf95ea30fe857253f4d055fc8fb/src/inspector/string-util.cc [modify] https://crrev.com/aed22ad12f66fdf95ea30fe857253f4d055fc8fb/src/inspector/string-util.h [add] https://crrev.com/aed22ad12f66fdf95ea30fe857253f4d055fc8fb/test/inspector/debugger/protocol-string-to-double-locale-expected.txt [add] https://crrev.com/aed22ad12f66fdf95ea30fe857253f4d055fc8fb/test/inspector/debugger/protocol-string-to-double-locale.js [modify] https://crrev.com/aed22ad12f66fdf95ea30fe857253f4d055fc8fb/third_party/inspector_protocol/lib/Parser_cpp.template
,
Apr 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3b3ae152b652a7a4240954a07172bd57bdd64a00 commit 3b3ae152b652a7a4240954a07172bd57bdd64a00 Author: kozyatinskiy <kozyatinskiy@chromium.org> Date: Fri Apr 28 03:46:58 2017 Roll third_party/inspector_protocol to efefa86c3183d307f0a0e53bf568fe57c5b58849 This roll includes: - [inspector_protocol] added StringUtil::toDouble method as requirement [1] [1] https://codereview.chromium.org/2843223005/ BUG= 712610 R=dgozman@chromium.org Review-Url: https://codereview.chromium.org/2846113002 Cr-Commit-Position: refs/heads/master@{#467878} [modify] https://crrev.com/3b3ae152b652a7a4240954a07172bd57bdd64a00/components/ui_devtools/string_util.h [modify] https://crrev.com/3b3ae152b652a7a4240954a07172bd57bdd64a00/content/browser/devtools/protocol_string.h [modify] https://crrev.com/3b3ae152b652a7a4240954a07172bd57bdd64a00/third_party/WebKit/Source/core/inspector/V8InspectorString.h [modify] https://crrev.com/3b3ae152b652a7a4240954a07172bd57bdd64a00/third_party/inspector_protocol/README.chromium [modify] https://crrev.com/3b3ae152b652a7a4240954a07172bd57bdd64a00/third_party/inspector_protocol/lib/Parser_cpp.template
,
Apr 28 2017
,
Apr 28 2017
Should be fixed in ToT and will be available in Canary soon. Let's merge it at least into beta since it potentially can affect headless or chrome driver. We need to merge both parts - V8 one and blink one.
,
Apr 29 2017
This bug requires manual review: DEPS changes referenced in bugdroid comments. Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 1 2017
Can you please confirm if this has been baked into canary and is this a safe merge? Is there enough automated test coverage for this?
,
May 1 2017
It hasn't reached Canary yet. I think it will be available in today Canary. There is a lot of automated test coverage for related code with US locale and minimum automated coverage for non-US locale. I tested manually with non-US and it works, and code by itself doesn't look dangerous.
,
May 1 2017
Thanks for the information - let's wait until this has been tested in Canary first and then request a review. We can't merge this in Beta until it has been atleast tested in Canary first.
,
May 1 2017
Issue 717216 has been merged into this issue.
,
May 3 2017
kozyatinskiy@ - Has this been tested in canary yet?
,
May 3 2017
It was tested in canary in at least two canary builds, looks like no issue was reported.
,
May 3 2017
Approving this for M59 based on comments 11, and 15.
,
May 4 2017
Tested the issue on Mac OS X 10.12.4 using Chrome Dev# 60.0.3088.3 and found the issue to be fixed. Hence adding TE-Verified labels. Adding screen cast for reference. Thank You.
,
May 8 2017
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 8 2017
Issue 717216 was merged into this one but I'm still able to reproduce it on 60.0.3089.0, should that issue be addressed by this fix as well? Thanks.
,
May 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/96f98cf3f6b43d5c1c4b43ed50f72cb0c8a85507 commit 96f98cf3f6b43d5c1c4b43ed50f72cb0c8a85507 Author: Alexey Kozyatinskiy <kozyatinskiy@chromium.org> Date: Tue May 09 00:19:41 2017 Roll third_party/inspector_protocol to efefa86c3183d307f0a0e53bf568fe57c5b58849 This roll includes: - [inspector_protocol] added StringUtil::toDouble method as requirement [1] [1] https://codereview.chromium.org/2843223005/ BUG= 712610 R=dgozman@chromium.org Review-Url: https://codereview.chromium.org/2846113002 Cr-Commit-Position: refs/heads/master@{#467878} (cherry picked from commit 3b3ae152b652a7a4240954a07172bd57bdd64a00) Review-Url: https://codereview.chromium.org/2866213002 . Cr-Commit-Position: refs/branch-heads/3071@{#474} Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641} [modify] https://crrev.com/96f98cf3f6b43d5c1c4b43ed50f72cb0c8a85507/components/ui_devtools/string_util.h [modify] https://crrev.com/96f98cf3f6b43d5c1c4b43ed50f72cb0c8a85507/content/browser/devtools/protocol_string.h [modify] https://crrev.com/96f98cf3f6b43d5c1c4b43ed50f72cb0c8a85507/third_party/WebKit/Source/core/inspector/V8InspectorString.h [modify] https://crrev.com/96f98cf3f6b43d5c1c4b43ed50f72cb0c8a85507/third_party/inspector_protocol/README.chromium [modify] https://crrev.com/96f98cf3f6b43d5c1c4b43ed50f72cb0c8a85507/third_party/inspector_protocol/lib/Parser_cpp.template
,
May 10 2017
Verified this issue on Mac 10.12.4 using chrome latest beta M59 #59.0.3071.47 by following steps mentioned in the original comment. Observed the elements on the Webpages is highlighting now. Hence adding the TE- Verified label Please refer the screen cast |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by caseq@chromium.org
, Apr 18 2017Status: Assigned (was: Unconfirmed)