Skip to content

Commit 6d890de

Browse files
dcroussodcrousso
committed
Web Inspector: Console: internal properties are not greyed out in object previews
https://bugs.webkit.org/show_bug.cgi?id=255385 Reviewed by Patrick Angle. We should have a consistent style between previews and actual remmote object views. Otherwise, internal properties appear as regular properties in the preview, but then look different when the object view is expanded. * Source/JavaScriptCore/inspector/protocol/Runtime.json: * Source/JavaScriptCore/inspector/InjectedScriptSource.js: (RemoteObject.prototype._appendPropertyPreview.appendPreview): * Source/WebInspectorUI/UserInterface/Models/PropertyPreview.js: (WI.PropertyPreview): (WI.PropertyPreview.fromPayload): (WI.PropertyPreview.prototype.get private): Added. Include an `isPrivate` property in `Runtime.PropertyPreview`. Drive-by: Refactor `WI.PropertyPreview` to put all optional properties into a `{...} = {}`. * Source/WebInspectorUI/UserInterface/Views/ObjectPreviewView.js: (WI.ObjectPreviewView.prototype._appendPropertyPreviews): * Source/WebInspectorUI/UserInterface/Views/ObjectPreviewView.css: (.object-preview .name:is(.private, .internal)): Added. Style private and internal property previews. * LayoutTests/inspector/runtime/getPreview.html: * LayoutTests/inspector/runtime/getPreview-expected.txt: Canonical link: https://commits.webkit.org/262924@main
1 parent 020a590 commit 6d890de

7 files changed

Lines changed: 54 additions & 5 deletions

File tree

‎LayoutTests/inspector/runtime/getPreview-expected.txt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,12 @@ PASS: RemoteObject.updatePreview should return null for a null RemoteObject.
4545
-- Running test case: Runtime.getPreview.Function
4646
{"type":"function","description":"function f() {}","lossless":true}
4747

48+
-- Running test case: Runtime.getPreview.Private
49+
{"type":"object","description":"Foo","lossless":false,"properties":[{"name":"#value","type":"number","value":"42","isPrivate":true}]}
50+
51+
-- Running test case: Runtime.getPreview.Internal
52+
{"type":"object","description":"EventTarget","lossless":false,"properties":[{"name":"listeners","type":"object","value":"Object","internal":true},{"name":"addEventListener","type":"function","value":""},{"name":"removeEventListener","type":"function","value":""},{"name":"dispatchEvent","type":"function","value":""}]}
53+
4854
-- Running test case: Runtime.getPreview.InvalidObjectId
4955
PASS: Should produce an error.
5056
Error: Missing injected script for given objectId

‎LayoutTests/inspector/runtime/getPreview.html‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,30 @@
140140
}
141141
});
142142

143+
suite.addTestCase({
144+
name: "Runtime.getPreview.Private",
145+
test(resolve, reject) {
146+
InspectorTest.evaluateInPage(`class Foo { #value = 42; } new Foo`, (error, remoteObject) => {
147+
RuntimeAgent.getPreview(remoteObject.objectId, (error, result) => {
148+
InspectorTest.log(JSON.stringify(result));
149+
resolve();
150+
});
151+
});
152+
}
153+
});
154+
155+
suite.addTestCase({
156+
name: "Runtime.getPreview.Internal",
157+
test(resolve, reject) {
158+
InspectorTest.evaluateInPage(`var EventTarget; target.addEventListener("click", console.log); target`, (error, remoteObject) => {
159+
RuntimeAgent.getPreview(remoteObject.objectId, (error, result) => {
160+
InspectorTest.log(JSON.stringify(result));
161+
resolve();
162+
});
163+
});
164+
}
165+
});
166+
143167
// ------
144168

145169
suite.addTestCase({

‎Source/JavaScriptCore/inspector/InjectedScriptSource.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1310,6 +1310,9 @@ let RemoteObject = class RemoteObject extends PrototypelessObjectBase
13101310
return InjectedScript.PropertyFetchAction.Stop;
13111311
}
13121312

1313+
if (descriptor.isPrivate)
1314+
property.isPrivate = true;
1315+
13131316
if (internal)
13141317
property.internal = true;
13151318

‎Source/JavaScriptCore/inspector/protocol/Runtime.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
{ "name": "subtype", "type": "string", "optional": true, "enum": ["array", "null", "node", "regexp", "date", "error", "map", "set", "weakmap", "weakset", "iterator", "class", "proxy"], "description": "Object subtype hint. Specified for <code>object</code> type values only." },
5050
{ "name": "value", "type": "string", "optional": true, "description": "User-friendly property value string." },
5151
{ "name": "valuePreview", "$ref": "ObjectPreview", "optional": true, "description": "Nested value preview." },
52+
{ "name": "isPrivate", "type": "boolean", "optional": true, "description": "True if this is a private field." },
5253
{ "name": "internal", "type": "boolean", "optional": true, "description": "True if this is an internal property." }
5354
]
5455
},

‎Source/WebInspectorUI/UserInterface/Models/PropertyPreview.js‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,11 @@
2525

2626
WI.PropertyPreview = class PropertyPreview
2727
{
28-
constructor(name, type, subtype, value, valuePreview, isInternalProperty)
28+
constructor(name, type, {subtype, value, valuePreview, isPrivateProperty, isInternalProperty} = {})
2929
{
3030
console.assert(typeof name === "string");
3131
console.assert(type);
32+
console.assert(!subtype || typeof subtype === "string");
3233
console.assert(!value || typeof value === "string");
3334
console.assert(!valuePreview || valuePreview instanceof WI.ObjectPreview);
3435

@@ -37,6 +38,7 @@ WI.PropertyPreview = class PropertyPreview
3738
this._subtype = subtype;
3839
this._value = value;
3940
this._valuePreview = valuePreview;
41+
this._private = isPrivateProperty;
4042
this._internal = isInternalProperty;
4143
}
4244

@@ -45,10 +47,13 @@ WI.PropertyPreview = class PropertyPreview
4547
// Runtime.PropertyPreview.
4648
static fromPayload(payload)
4749
{
48-
if (payload.valuePreview)
49-
payload.valuePreview = WI.ObjectPreview.fromPayload(payload.valuePreview);
50-
51-
return new WI.PropertyPreview(payload.name, payload.type, payload.subtype, payload.value, payload.valuePreview, payload.internal);
50+
return new WI.PropertyPreview(payload.name, payload.type, {
51+
subtype: payload.subtype,
52+
value: payload.value,
53+
valuePreview: payload.valuePreview ? WI.ObjectPreview.fromPayload(payload.valuePreview) : undefined,
54+
isPrivateProperty: payload.isPrivate,
55+
isInternalProperty: payload.internal,
56+
});
5257
}
5358

5459
// Public
@@ -58,5 +63,6 @@ WI.PropertyPreview = class PropertyPreview
5863
get subtype() { return this._subtype; }
5964
get value() { return this._value; }
6065
get valuePreview() { return this._valuePreview; }
66+
get private() { return this._private; }
6167
get internal() { return this._internal; }
6268
};

‎Source/WebInspectorUI/UserInterface/Views/ObjectPreviewView.css‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@
3939
color: hsl(295, 76%, 32%);
4040
}
4141

42+
.object-preview .name:is(.private, .internal) {
43+
color: var(--text-color-secondary);
44+
}
45+
4246
.object-preview > .size {
4347
font-style: normal;
4448
color: var(--console-secondary-text-color);

‎Source/WebInspectorUI/UserInterface/Views/ObjectPreviewView.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,11 @@ WI.ObjectPreviewView = class ObjectPreviewView extends WI.Object
230230
let nameElement = element.appendChild(document.createElement("span"));
231231
nameElement.className = "name";
232232
nameElement.textContent = property.name;
233+
if (property.private)
234+
nameElement.classList.add("private");
235+
if (property.internal)
236+
nameElement.classList.add("internal");
237+
233238
element.append(": ");
234239
}
235240

0 commit comments

Comments
 (0)