Bug 51528 - [v8] ScriptController::evaluate returns empty result for undefined. Can't distinguish errors from undefined results.
Summary: [v8] ScriptController::evaluate returns empty result for undefined. Can't dis...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-12-23 03:02 PST by Tom R
Modified: 2011-03-01 01:00 PST (History)
3 users (show)

See Also:


Attachments
fix, allow the "undefined" v8 object to be returned (511 bytes, patch)
2010-12-23 06:39 PST, Tom R
eric: review-
Details | Formatted Diff | Diff
Patch (1.20 KB, patch)
2011-02-22 12:04 PST, anthony taranto
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tom R 2010-12-23 03:02:15 PST
ScriptController::evaluate contains:

    if (object.IsEmpty() || object->IsUndefined())
        return ScriptValue();

Should be:

    if (object.IsEmpty())
        return ScriptValue();

Returning an empty ScriptValue should indicate an error. As undefined is a valid return value for void calls returning an empty ScriptValue makes it impossible to distinguish undefined results from errors.

Related mailing list thread:

https://lists.webkit.org/pipermail/webkit-dev/2010-December/015417.html
Comment 1 Tom R 2010-12-23 06:39:17 PST
Created attachment 77331 [details]
fix, allow the "undefined" v8 object to be returned
Comment 2 Marshall Greenblatt 2011-01-07 10:29:08 PST
Hi Yury, please review this patch.
Comment 3 Eric Seidel (no email) 2011-01-11 03:08:49 PST
Comment on attachment 77331 [details]
fix, allow the "undefined" v8 object to be returned

This patch was not created correctly.  Please create from a WebKit checkout.  Also, all patches require a ChangeLog.

webkit-patch upload

will walk you through much of this.
Comment 4 Yury Semikhatsky 2011-01-11 03:30:55 PST
Hi Tom, please add proper ChangeLog entry to the patch as Eric suggested, otherwise the change looks good to me.

(In reply to comment #2)
> Hi Yury, please review this patch.
Comment 5 anthony taranto 2011-02-22 12:04:55 PST
Created attachment 83357 [details]
Patch
Comment 6 anthony taranto 2011-02-22 12:06:50 PST
Generated a patch with webkit-patch upload and included a ChangeLog. Let me know if anything else is needed.
Comment 7 Marshall Greenblatt 2011-02-25 12:16:56 PST
Can someone add this patch to the commit queue? Thanks.
Comment 8 Yury Semikhatsky 2011-03-01 01:00:11 PST
Comment on attachment 83357 [details]
Patch

Clearing flags on attachment: 83357

Committed r79981: <http://trac.webkit.org/changeset/79981>
Comment 9 Yury Semikhatsky 2011-03-01 01:00:18 PST
All reviewed patches have been landed.  Closing bug.