Use QJSValue as method parameter type for the scripting interface

Authored by fvogt on Aug 14 2018, 9:52 AM.



If a slot or Q_INVOKABLE has a QVariant as parameter and gets called
from a QJSEngine's script, it receives a QJSValue wrapped as QVariant.
To get a QVariant with the actual value wrapped, calling QJSValue::toVariant
is necessary.

I'm not entirely sure whether this is intentional behaviour of QJSEngine, but
even if it's a bug we'll have to workaround it.

BUG: 397338

Test Plan

I have favorites in kickoff again.

Diff Detail

R120 Plasma Workspace
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
fvogt created this revision.Aug 14 2018, 9:52 AM
Restricted Application added a project: Plasma. · View Herald TranscriptAug 14 2018, 9:52 AM
fvogt requested review of this revision.Aug 14 2018, 9:52 AM
anthonyfieroni added inline comments.

So writeEntry gets QJSValue& now why you call again toVariant(), here everywhere else.

fvogt added inline comments.Aug 14 2018, 11:00 AM

KConfigGroup != ConfigGroup. d->configGroup is from kconfig and does not take QJSValue. That's the core of the bugfix.

A different approach of fixing this is to do something like

if(value.userType() == qMetaTypeId<QJSValue>())
    value = value.value<QJSValue>().toVariant();

in every function for every QVariant parameter. Advantages are that it's more obvious and the function's interface does not change.
Disadvantages are that it has more overhead and more lines of code.

mart added a subscriber: mart.Aug 20 2018, 12:57 PM
mart added inline comments.

maybe it needs to return a QJSValue as well then?

fvogt added inline comments.Aug 20 2018, 1:04 PM

No, that works fine.

My theory is that it works because as a QVariant<QJSValue> can still be converted to QString, int etc.
KConfig accesses the meta type ID directly (which is always the user type ID of QJSValue) and doesn't check for "is_convertible" or similar, so does not know what to do with it.

davidedmundson accepted this revision.Aug 22 2018, 12:18 PM
This revision is now accepted and ready to land.Aug 22 2018, 12:18 PM
This revision was automatically updated to reflect the committed changes.