Summary: | Web Inspector: console.log fires getters for deep properties | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brandon Dail <kierkegaurd> | ||||
Component: | Web Inspector | Assignee: | Joseph Pecoraro <joepeck> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bburg, commit-queue, ews-watchlist, inspector-bugzilla-changes, joepeck, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | Safari 11 | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Brandon Dail
2018-07-10 17:33:27 PDT
Interesting that this happens when logging `outer` but not `inner`. Seems then it might be an accident in the property preview path. Switching to `console.log` to `console.trace` gives us: [Log] Trace: getter fired Console Evaluation (Console Evaluation 1:6) _isPreviewableObjectInternal _appendPropertyPreviews _generatePreview RemoteObject create log Console Evaluation (Console Evaluation 1:13) evaluateWithScopeExtension _evaluateOn _evaluateAndWrap It looks like at the bottom of _isPreviewableObjectInternal we access properties `object[propertyName]` without regard to whether or not it may be a getter. _isPreviewableObjectInternal(object, knownObjects, depth) { ... // Objects are simple if they have 3 or less simple properties. let ownPropertyNames = Object.getOwnPropertyNames(object); if (ownPropertyNames.length > 3) return false; for (let i = 0; i < ownPropertyNames.length; ++i) { let propertyName = ownPropertyNames[i]; if (!this._isPreviewableObjectInternal(object[propertyName], knownObjects, depth)) return false; } return true; } Created attachment 346936 [details]
[PATCH] Proposed Fix
Comment on attachment 346936 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=346936&action=review > Source/JavaScriptCore/inspector/InjectedScriptSource.js:1295 > + let descriptor = Object.getOwnPropertyDescriptor(object, propertyName); Might be worth verifying that we're ok w/ this doing effects? Like I believe this calls a trap in a proxy. (In reply to Saam Barati from comment #5) > Comment on attachment 346936 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=346936&action=review > > > Source/JavaScriptCore/inspector/InjectedScriptSource.js:1295 > > + let descriptor = Object.getOwnPropertyDescriptor(object, propertyName); > > Might be worth verifying that we're ok w/ this doing effects? Like I believe > this calls a trap in a proxy. Is there a way to tell if its a getter without an observable effect (without being a built-in)? Comment on attachment 346936 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=346936&action=review r=me >>> Source/JavaScriptCore/inspector/InjectedScriptSource.js:1295 >>> + let descriptor = Object.getOwnPropertyDescriptor(object, propertyName); >> >> Might be worth verifying that we're ok w/ this doing effects? Like I believe this calls a trap in a proxy. > > Is there a way to tell if its a getter without an observable effect (without being a built-in)? Nope, not in a general way. Comment on attachment 346936 [details] [PATCH] Proposed Fix Clearing flags on attachment: 346936 Committed r234780: <https://trac.webkit.org/changeset/234780> All reviewed patches have been landed. Closing bug. |