Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Web Inspector: Console: internal properties are not greyed out in obj…
…ect 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
  • Loading branch information
dcrousso committed Apr 13, 2023
1 parent 020a590 commit 6d890de
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 5 deletions.
6 changes: 6 additions & 0 deletions LayoutTests/inspector/runtime/getPreview-expected.txt
Expand Up @@ -45,6 +45,12 @@ PASS: RemoteObject.updatePreview should return null for a null RemoteObject.
-- Running test case: Runtime.getPreview.Function
{"type":"function","description":"function f() {}","lossless":true}

-- Running test case: Runtime.getPreview.Private
{"type":"object","description":"Foo","lossless":false,"properties":[{"name":"#value","type":"number","value":"42","isPrivate":true}]}

-- Running test case: Runtime.getPreview.Internal
{"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":""}]}

-- Running test case: Runtime.getPreview.InvalidObjectId
PASS: Should produce an error.
Error: Missing injected script for given objectId
Expand Down
24 changes: 24 additions & 0 deletions LayoutTests/inspector/runtime/getPreview.html
Expand Up @@ -140,6 +140,30 @@
}
});

suite.addTestCase({
name: "Runtime.getPreview.Private",
test(resolve, reject) {
InspectorTest.evaluateInPage(`class Foo { #value = 42; } new Foo`, (error, remoteObject) => {
RuntimeAgent.getPreview(remoteObject.objectId, (error, result) => {
InspectorTest.log(JSON.stringify(result));
resolve();
});
});
}
});

suite.addTestCase({
name: "Runtime.getPreview.Internal",
test(resolve, reject) {
InspectorTest.evaluateInPage(`var target = new EventTarget; target.addEventListener("click", console.log); target`, (error, remoteObject) => {
RuntimeAgent.getPreview(remoteObject.objectId, (error, result) => {
InspectorTest.log(JSON.stringify(result));
resolve();
});
});
}
});

// ------

suite.addTestCase({
Expand Down
3 changes: 3 additions & 0 deletions Source/JavaScriptCore/inspector/InjectedScriptSource.js
Expand Up @@ -1310,6 +1310,9 @@ let RemoteObject = class RemoteObject extends PrototypelessObjectBase
return InjectedScript.PropertyFetchAction.Stop;
}

if (descriptor.isPrivate)
property.isPrivate = true;

if (internal)
property.internal = true;

Expand Down
1 change: 1 addition & 0 deletions Source/JavaScriptCore/inspector/protocol/Runtime.json
Expand Up @@ -49,6 +49,7 @@
{ "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." },
{ "name": "value", "type": "string", "optional": true, "description": "User-friendly property value string." },
{ "name": "valuePreview", "$ref": "ObjectPreview", "optional": true, "description": "Nested value preview." },
{ "name": "isPrivate", "type": "boolean", "optional": true, "description": "True if this is a private field." },
{ "name": "internal", "type": "boolean", "optional": true, "description": "True if this is an internal property." }
]
},
Expand Down
16 changes: 11 additions & 5 deletions Source/WebInspectorUI/UserInterface/Models/PropertyPreview.js
Expand Up @@ -25,10 +25,11 @@

WI.PropertyPreview = class PropertyPreview
{
constructor(name, type, subtype, value, valuePreview, isInternalProperty)
constructor(name, type, {subtype, value, valuePreview, isPrivateProperty, isInternalProperty} = {})
{
console.assert(typeof name === "string");
console.assert(type);
console.assert(!subtype || typeof subtype === "string");
console.assert(!value || typeof value === "string");
console.assert(!valuePreview || valuePreview instanceof WI.ObjectPreview);

Expand All @@ -37,6 +38,7 @@ WI.PropertyPreview = class PropertyPreview
this._subtype = subtype;
this._value = value;
this._valuePreview = valuePreview;
this._private = isPrivateProperty;
this._internal = isInternalProperty;
}

Expand All @@ -45,10 +47,13 @@ WI.PropertyPreview = class PropertyPreview
// Runtime.PropertyPreview.
static fromPayload(payload)
{
if (payload.valuePreview)
payload.valuePreview = WI.ObjectPreview.fromPayload(payload.valuePreview);

return new WI.PropertyPreview(payload.name, payload.type, payload.subtype, payload.value, payload.valuePreview, payload.internal);
return new WI.PropertyPreview(payload.name, payload.type, {
subtype: payload.subtype,
value: payload.value,
valuePreview: payload.valuePreview ? WI.ObjectPreview.fromPayload(payload.valuePreview) : undefined,
isPrivateProperty: payload.isPrivate,
isInternalProperty: payload.internal,
});
}

// Public
Expand All @@ -58,5 +63,6 @@ WI.PropertyPreview = class PropertyPreview
get subtype() { return this._subtype; }
get value() { return this._value; }
get valuePreview() { return this._valuePreview; }
get private() { return this._private; }
get internal() { return this._internal; }
};
Expand Up @@ -39,6 +39,10 @@
color: hsl(295, 76%, 32%);
}

.object-preview .name:is(.private, .internal) {
color: var(--text-color-secondary);
}

.object-preview > .size {
font-style: normal;
color: var(--console-secondary-text-color);
Expand Down
Expand Up @@ -230,6 +230,11 @@ WI.ObjectPreviewView = class ObjectPreviewView extends WI.Object
let nameElement = element.appendChild(document.createElement("span"));
nameElement.className = "name";
nameElement.textContent = property.name;
if (property.private)
nameElement.classList.add("private");
if (property.internal)
nameElement.classList.add("internal");

element.append(": ");
}

Expand Down

0 comments on commit 6d890de

Please sign in to comment.