[webkit-reviews] review denied: [Bug 28078] WebInspector: Move object properties read / write access into InjectedScript : [Attachment 34309] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Aug 7 13:01:35 PDT 2009


Timothy Hatcher <timothy at hatcher.name> has denied Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 28078: WebInspector: Move object properties read / write access into
InjectedScript
https://bugs.webkit.org/show_bug.cgi?id=28078

Attachment 34309: patch
https://bugs.webkit.org/attachment.cgi?id=34309&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>

> -WebInspector.ObjectPropertiesSection = function(object, title, subtitle,
emptyPlaceholder, ignoreHasOwnProperty, extraProperties,
treeElementConstructor)
> +WebInspector.ObjectRef = function(objectId, path, protoDepth)

I think this should go into a new file and not ObjectPropertiesSection.js,
since it is used in other places and not specific to the
ObjectPropertiesSection.

I am not too fond of the Ref suffix. Do you like ObjectProxy better?

> +    this.objectId = objectId;

You should add a comment about this, this looks temporary.

> +    this.path = path || [];

> +InjectedScript._objectForId = function(objectId)
> +{
> +    return objectId;
> +}

You hould add a comment about this being temporary.


More information about the webkit-reviews mailing list