RESOLVED FIXED 40067
[Qt] The QScriptValuePrivate::CSpecial is too generic. 
https://bugs.webkit.org/show_bug.cgi?id=40067
Summary [Qt] The QScriptValuePrivate::CSpecial is too generic.
Jędrzej Nowacki
Reported 2010-06-02 09:26:57 PDT
The QScriptValuePrivate::CSpecial should be divided into a separate states; CNull and CUndefined. It would simplify the QScriptValue's design and implementation which should use the m_state member for distinguish between states not m_value.
Attachments
Fix v1 (5.80 KB, patch)
2010-06-02 09:35 PDT, Jędrzej Nowacki 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Jędrzej Nowacki
Comment 1 2010-06-02 09:35:37 PDT
Created attachment 57663 [details] Fix v1 No functional changes.
Kenneth Rohde Christiansen
Comment 2 2010-06-02 11:44:23 PDT
Comment on attachment 57663 [details] Fix v1 JavaScriptCore/qt/api/qscriptvalue_p.h:54 + CUndefined -> QSVP is undefined, but a JSC engine hasn't been associated yet. So does this not change the behaviour of QtScript?
Jędrzej Nowacki
Comment 3 2010-06-03 00:16:46 PDT
(In reply to comment #2) > (From update of attachment 57663 [details]) > JavaScriptCore/qt/api/qscriptvalue_p.h:54 > + CUndefined -> QSVP is undefined, but a JSC engine hasn't been associated yet. > So does this not change the behaviour of QtScript? True, It is a small design fix :-) The CSpecial state was a bad idea. From the beginning CNull and CUndefined should be separated. Patch removes conversions between double and enum. > - case CSpecial: > - return m_number == static_cast<int>(QScriptValue::NullValue); > + case CNull: > + return true; Code is simpler, and there is no need to check other variable then m_state to find the real state of QScriptValuePrivate > - case CSpecial: > - return m_number == QScriptValue::NullValue ? QString::fromLatin1("null") : QString::fromLatin1("undefined"); > + case CNull: > + return QString::fromLatin1("null"); > + case CUndefined: > + return QString::fromLatin1("undefined"); I planed to change it in Feb. but the patch touches too many functions in the QSVP, so it was difficult to apply it without conflicts. This is the first time when a QSVP patches are not waiting in bugzilla's commit queue.
WebKit Commit Bot
Comment 4 2010-06-03 22:57:52 PDT
Comment on attachment 57663 [details] Fix v1 Clearing flags on attachment: 57663 Committed r60655: <http://trac.webkit.org/changeset/60655>
WebKit Commit Bot
Comment 5 2010-06-03 22:57:58 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.