Bug 65518

Summary: Web Inspector: there should be a way to tell what properties are non-enumerable when expanding objects.
Product: WebKit Reporter: Pavel Feldman <pfeldman>
Component: Web Inspector (Deprecated)Assignee: Pavel Feldman <pfeldman>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, bweinstein, joepeck, keishi, loislo, ossy, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 65905    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
[IMAGE] Looks with patch applied. none

Description Pavel Feldman 2011-08-01 23:21:00 PDT
<user report>
Comment 1 Pavel Feldman 2011-08-09 01:41:15 PDT
Created attachment 103336 [details]
Patch
Comment 2 Pavel Feldman 2011-08-09 01:41:39 PDT
Created attachment 103337 [details]
[IMAGE] Looks with patch applied.
Comment 3 WebKit Review Bot 2011-08-09 01:44:31 PDT
Attachment 103336 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1

Source/WebCore/inspector/InjectedScript.cpp:88:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InspectorRuntimeAgent.h:73:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InspectorRuntimeAgent.cpp:112:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScript.h:73:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 4 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Yury Semikhatsky 2011-08-09 01:57:19 PDT
Comment on attachment 103336 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=103336&action=review

> Source/WebCore/inspector/Inspector.json:245
> +                    { "name": "get", "$ref": "RemoteObject", "description": "A function which serves as a getter for the property, or <code>undefined</code> if there is no getter (accessor descriptors only)." },

get -> getter

> Source/WebCore/inspector/Inspector.json:246
> +                    { "name": "set", "$ref": "RemoteObject", "description": "A function which serves as a setter for the property, or <code>undefined</code> if there is no setter (accessor descriptors only)." },

set -> setter

> Source/WebCore/inspector/Inspector.json:296
> +                    { "name": "ownProperties", "optional": true, "type": "boolean", "description": "If true, returns properties belonging only to the element itself, not to its prototype chain." }

ownProperties -> ownPropertiesOnly ?

> Source/WebCore/inspector/front-end/RemoteObject.js:230
> +    this.writable = descriptor ? !!descriptor.writable : true;

Please remove !! before descriptor.writable
Comment 5 Pavel Feldman 2011-08-09 02:01:35 PDT
(In reply to comment #4)
> (From update of attachment 103336 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=103336&action=review
> 
> > Source/WebCore/inspector/Inspector.json:245
> > +                    { "name": "get", "$ref": "RemoteObject", "description": "A function which serves as a getter for the property, or <code>undefined</code> if there is no getter (accessor descriptors only)." },
> 
> get -> getter
> 

I agree here, but I think sticking with the ECMA spec is more important.

> > Source/WebCore/inspector/Inspector.json:246
> > +                    { "name": "set", "$ref": "RemoteObject", "description": "A function which serves as a setter for the property, or <code>undefined</code> if there is no setter (accessor descriptors only)." },
> 
> set -> setter

ditto

> 
> > Source/WebCore/inspector/Inspector.json:296
> > +                    { "name": "ownProperties", "optional": true, "type": "boolean", "description": "If true, returns properties belonging only to the element itself, not to its prototype chain." }
> 
> ownProperties -> ownPropertiesOnly ?
> 

I think present way is more clear, see the documentation field.

> > Source/WebCore/inspector/front-end/RemoteObject.js:230
> > +    this.writable = descriptor ? !!descriptor.writable : true;
> 
> Please remove !! before descriptor.writable

Done.
Comment 6 Pavel Feldman 2011-08-09 02:05:31 PDT
Committed r92670: <http://trac.webkit.org/changeset/92670>
Comment 8 Csaba Osztrogon√°c 2011-08-09 03:28:24 PDT
Reopen, because it was rolled out by https://trac.webkit.org/changeset/92671
Comment 9 Pavel Feldman 2011-08-10 06:41:51 PDT
Committed r92765: <http://trac.webkit.org/changeset/92765>