Bug 28665 - Inspector: Throws an Error on "null"
Summary: Inspector: Throws an Error on "null"
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-08-22 23:53 PDT by Joseph Pecoraro
Modified: 2009-08-23 14:42 PDT (History)
5 users (show)

See Also:


Attachments
[PATCH] One Possible Solution (1.23 KB, patch)
2009-08-23 00:04 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[PATCH] Pavel's JavaScript Approach (1.66 KB, patch)
2009-08-23 07:55 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2009-08-22 23:53:59 PDT
The Console is throwing an error on "null".  This seems to happen because after the result of an eval goes to InspectorController.wrapObject(), which doesn't handle null very well.

Current Behavior:

> null
TypeError: Result of expression 'wrapper' [null] is not an object.
> [null, null]
[]
> var o = { a: null }
Object
  a: null
> o.a
TypeError: Result of expression 'wrapper' [null] is not an object.


Expected Behavior (from Safari 4.0.3):

> null
null
> [null, null]
[null, null]
> var o = { a:null }
undefined
> o
Object
  a: null
> o.a
null
Comment 1 Joseph Pecoraro 2009-08-23 00:04:03 PDT
Created attachment 38447 [details]
[PATCH] One Possible Solution

No other falsey value causes wrapObject() to return a null value (NaN, undefined, 0, '').  This solution tackles null before calling wrapObject.  Another solution could be checking the result of wrapObject (wrapper).  And yet another solution could handle null differently inside InspectorController.wrapObject().  This approach seemed simplest.

Pavel, would there be a more appropriate solution?
Comment 2 Joseph Pecoraro 2009-08-23 00:08:45 PDT
Actually that "exception: false" part isn't necessary, because thats the implicit default if its left out.  So I can remove that if this is an okay way to go.
Comment 3 Pavel Feldman 2009-08-23 02:11:03 PDT
> +            return { value: InjectedScript.createProxyObject(null, null), exception: false };

We don't wrap primitive values with proxy objects at the moment, why wrapping null. I would just do

    return { value: null };

However, that would require fixing ConsoleView's proxy detection:

    var isProxy = typeof output === "object";

should be replaced with

    var isProxy = output !== null && typeof output === "object";
Comment 4 Pavel Feldman 2009-08-23 02:26:53 PDT
Additional note: fixing InspectorController.wrapObject would be nicer, but you would need to do that in both bindings (comparing to null are returning argument). It will still require mentioned fix in the console though.
Comment 5 Joseph Pecoraro 2009-08-23 07:55:23 PDT
Created attachment 38453 [details]
[PATCH] Pavel's JavaScript Approach
Comment 6 Joseph Pecoraro 2009-08-23 07:58:49 PDT
(In reply to comment #4)
> Additional note: fixing InspectorController.wrapObject would be nicer, but you
> would need to do that in both bindings (comparing to null are returning
> argument). It will still require mentioned fix in the console though.

What I said earlier was not accurate.  wrapObject doesn't have trouble with null, it just ends up returning the value passed in, which was null, and a null value isn't handled well afterwards.  I like this approach.
Comment 7 Eric Seidel (no email) 2009-08-23 14:42:15 PDT
Comment on attachment 38453 [details]
[PATCH] Pavel's JavaScript Approach

Clearing flags on attachment: 38453

Committed r47696: <http://trac.webkit.org/changeset/47696>
Comment 8 Eric Seidel (no email) 2009-08-23 14:42:18 PDT
All reviewed patches have been landed.  Closing bug.