Skip to content

Conversation

@dcrousso
Copy link
Member

@dcrousso dcrousso commented Apr 13, 2023

6d890de

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

f6c7bcc

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug 🧪 wpe-wk2
✅ 🧪 webkitperl 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 🧪 gtk-wk2
🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ❌ 🧪 api-gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 tv 🧪 mac-AS-debug-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 jsc-armv7-tests
✅ 🛠 🧪 merge ✅ 🛠 watch ✅ 🛠 jsc-mips
✅ 🛠 watch-sim ✅ 🧪 jsc-mips-tests

@dcrousso dcrousso requested a review from patrickangle as a code owner April 13, 2023 03:51
@dcrousso dcrousso self-assigned this Apr 13, 2023
@dcrousso dcrousso added the Web Inspector Bugs related to the WebKit Web Inspector. label Apr 13, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 13, 2023
Copy link
Contributor

@patrickangle patrickangle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me once we have a set of test passes. Looks like a lot of tests were relying on PropertyPreview having undefined values for various things instead of null/false default values and need rebaselined or updated to not output those nulls and falses 😓

@dcrousso dcrousso removed the merging-blocked Applied to prevent a change from being merged label Apr 13, 2023
@dcrousso dcrousso added the merge-queue Applied to send a pull request to merge-queue label Apr 13, 2023
…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
@webkit-commit-queue
Copy link
Collaborator

Committed 262924@main (6d890de): https://commits.webkit.org/262924@main

Reviewed commits have been landed. Closing PR #12685 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 6d890de into WebKit:main Apr 13, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Apr 13, 2023
@dcrousso dcrousso deleted the eng/255385 branch April 13, 2023 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Web Inspector Bugs related to the WebKit Web Inspector.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants