Bug 35387 - [Qt] Sometimes QScriptValue::isObject returns an incorrect value
Summary: [Qt] Sometimes QScriptValue::isObject returns an incorrect value
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Jędrzej Nowacki
URL:
Keywords: Qt
Depends on: 34850
Blocks: 31863
  Show dependency treegraph
 
Reported: 2010-02-25 09:04 PST by Jędrzej Nowacki
Modified: 2010-03-03 21:02 PST (History)
3 users (show)

See Also:


Attachments
Fix v1 (37.49 KB, patch)
2010-02-25 09:08 PST, Jędrzej Nowacki
no flags Details | Formatted Diff | Diff
Fix v2 (37.47 KB, patch)
2010-02-25 09:46 PST, Jędrzej Nowacki
no flags Details | Formatted Diff | Diff
Fix v3 (37.49 KB, patch)
2010-03-03 07:12 PST, Jędrzej Nowacki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jędrzej Nowacki 2010-02-25 09:04:25 PST
Example:

bool foo() 
{
    QScriptEngine engine;
    return engine.evaluate("1").isObject();
}

Function foo() returns true. It seems that isObject() function call automatically converts a primitive value to a corespondent object wrapper. Issue is visible only if the value is returned as result of evaluate call. This: 

bool foo() 
{
    return QScriptValue(1).isObject();
}

works correctly.
Comment 1 Jędrzej Nowacki 2010-02-25 09:08:31 PST
Created attachment 49493 [details]
Fix v1
Comment 2 Jędrzej Nowacki 2010-02-25 09:09:32 PST
Proposed patch depends on 34850
Comment 3 Kent Hansen 2010-02-25 09:20:38 PST
Looks good, except for the misspelling in the doc for refineJSValue ("retruns"), and I think it should say "Refines the state of this ... and returns the new state". No reason to use different wording in the description when the function already has a descriptive name.
Comment 4 Jędrzej Nowacki 2010-02-25 09:46:20 PST
Created attachment 49500 [details]
Fix v2

(In reply to comment #3)
> Looks good, except for the misspelling in the doc for refineJSValue
> ("retruns"), and I think it should say "Refines the state of this ... and
> returns the new state". No reason to use different wording in the description
> when the function already has a descriptive name.
Done.

As previous patch this one is dependent on bug 34850, that means it cant be applied right now. I have tried apply it on trunk;
~/dev/WebKit$ git apply -v /tmp/isObjectV2.diff
Checking patch JavaScriptCore/ChangeLog...
Checking patch JavaScriptCore/qt/api/qscriptvalue_p.h...
Checking patch JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated.cpp...
error: while searching for:
{
    QFETCH(bool, expected);
    QCOMPARE(value.isObject(), expected);
}

DEFINE_TEST_FUNCTION(isObject)

void tst_QScriptValue::toString_initData()
{
    QTest::addColumn<QString>("expected");

error: patch failed: JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated.cpp:552
error: JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated.cpp: patch does not apply
Comment 5 Jędrzej Nowacki 2010-03-03 07:12:53 PST
Created attachment 49903 [details]
Fix v3

Changes:
 - refineJSValue was renamed to refinedJSValue
 - fix typo in a comment.
Comment 6 WebKit Commit Bot 2010-03-03 21:02:13 PST
Comment on attachment 49903 [details]
Fix v3

Clearing flags on attachment: 49903

Committed r55502: <http://trac.webkit.org/changeset/55502>
Comment 7 WebKit Commit Bot 2010-03-03 21:02:17 PST
All reviewed patches have been landed.  Closing bug.