New issue
Advanced search Search tips

Issue 917136 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 28
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Regression: custom formatters are not always applied in ObjectPropertiesSection.js

Reported by antonin....@gmail.com, Dec 20

Issue description

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

Steps to reproduce the problem:
1. start with fresh devtools, enable custom formatters in devtools settings
2. `window.devtoolsFormatters = [{header: function(o) { return ["span", {style: "color:green"}, "CFH"] }, hasBody: function(o) {return false}}]`
3. `(function() {var x={}; debugger;})`

What is the expected behavior?
1. after step #2, you should see green CFH in console as return
2. after step #3, you should see 2x green CFH in Sources > Scope > Local, both `this` and `x` should have green CFH as their presented value

What went wrong?
In Sources > Scope > Local the custom formatter was not applied and displays usual value (`{}` in this case) 

Did this work before? Yes not sure, but the regression has been there for several weeks, I guess

Chrome version: 71.0.3578.98  Channel: stable
OS Version: OS X 10.14.3
Flash Version:
 
Ah, turns out I cannot edit my posts here. So here are corrections:

Step #2 should be executed in console.
Step #3 should be `(function() {var x={}; debugger;})()` (note trailing parens to exec the function)

In 'What went wrong' section I should note that only 'x' does not have custom formatter  applied.

Chrome version: 71.0.3578.98 is still good (that is the chrome I have a cookie with session to log into bugs.chromium.org). My current canary is bad: Google Chrome	73.0.3646.0 (Official Build) canary (64-bit) Revision	fe8cc77bcda70360146d09741601e582ce676de0-refs/branch-heads/3646@{#1}
In step 3 the code was missing () at the end (see the screenshots).

Bisected to 66daabcca996bf581d2a85b0d942a64fa7c43f07
"inspector: generate custom preview using native code"
Landed in 72.0.3590.0 via r601826

good.png
18.7 KB View Download
bad.png
21.8 KB View Download
Cc: pbomm...@chromium.org dgozman@chromium.org
Labels: Needs-Triage-M71
Labels: ReleaseBlock-Stable Target-72 M-72
Owner: alph@chromium.org
Status: Assigned (was: Unconfirmed)
@alph: let's check this for 72, and try to fix if it's broken there.
Friendly ping to look into this issue and to provide further update on this issue as it has been marked as a stable blocker.

Thanks!
Yes, it's broken in M-72
Cc: hablich@chromium.org
Labels: Merge-Request-7.2
Status: Fixed (was: Assigned)
Labels: OS-Linux OS-Windows
Labels: -Merge-Request-7.2 merge-approved-7.2
Project Member

Comment 11 by bugdroid1@chromium.org, Jan 7

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

commit 98fe792443a995d19ba6bc026ea421af54ddff6e
Author: Alexei Filippov <alph@chromium.org>
Date: Mon Jan 07 19:05:04 2019

Merged: [inspector] Make InjectedScript::getProperties respect custom formatters

Revision: 4eae3bb140717b96142cfa08dd202e7706903a0d

BUG= chromium:917136 
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=dgozman@chromium.org

Change-Id: I0cb9f109573968804711c85a169c8d9999b29a97
Reviewed-on: https://chromium-review.googlesource.com/c/1398722
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/branch-heads/7.2@{#33}
Cr-Branched-From: 6acd03c9b8a8232aee95f25fbf6ae822aaedae75-refs/heads/7.2.502@{#1}
Cr-Branched-From: b03041de094610ef24e0e4fb6bf4c700fa1553ed-refs/heads/master@{#57910}
[modify] https://crrev.com/98fe792443a995d19ba6bc026ea421af54ddff6e/src/inspector/injected-script.cc
[modify] https://crrev.com/98fe792443a995d19ba6bc026ea421af54ddff6e/src/inspector/injected-script.h
[modify] https://crrev.com/98fe792443a995d19ba6bc026ea421af54ddff6e/src/inspector/value-mirror.cc
[modify] https://crrev.com/98fe792443a995d19ba6bc026ea421af54ddff6e/src/inspector/value-mirror.h
[modify] https://crrev.com/98fe792443a995d19ba6bc026ea421af54ddff6e/test/inspector/runtime/custom-preview-expected.txt
[modify] https://crrev.com/98fe792443a995d19ba6bc026ea421af54ddff6e/test/inspector/runtime/custom-preview.js

Project Member

Comment 12 by sheriffbot@chromium.org, Jan 11

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
Labels: -merge-approved-7.2

Sign in to add a comment