Review Request 123789: automatically add newly installed plasmoids in the systray

[Marco Martin]

> On May 14, 2015, 2:19 p.m., David Edmundson wrote:
> > applets/systemtray/package/contents/ui/main.qml, line 126
> > <https://git.reviewboard.kde.org/r/123789/diff/3/?file=369034#file369034line126>
> >
> >     is it this thing?
> >     http://tsdgeos.blogspot.co.uk/2015/02/qt-54-qml-c-qvariant-issues.html
> >     
> >     Could you test if this https://paste.kde.org/pgu5pnukc on kdeclarative fixes it?
> 
> Marco Martin wrote:
>     seems it's that one.
>     i tried your patch, but with the qml
>     
>             if (extraItems.length > 0) {
>                 plasmoid.configuration.extraItems = plasmoid.configuration.extraItems.concat(extraItems);
>             }
>             if (newKnownItems.length > 0) {
>                 lasmoid.configuration.knownItems = plasmoid.configuration.knownItems.concat(knownItems);
>             }
>             
>     it still breaks the saved value
> 
> Marco Martin wrote:
>     i mean,
>     if (extraItems.length > 0) {
>                 plasmoid.configuration.extraItems = plasmoid.configuration.extraItems.concat(extraItems);
>             }
>             if (newKnownItems.length > 0) {
>                 plasmoid.configuration.knownItems = plasmoid.configuration.knownItems.concat(newKnownItems);
>             }
>             
>     but yeah, still broken

even more fun, this what a debug in ConfigPropertyMapPrivate::writeConfigValue says
qDebug()<<key<<value<<value.value<QJSValue>().toVariant();

becomes:

"knownItems" QVariant(QJSValue, ) QVariant(QVariantList, (QVariant(QStringList, ("", "org.kde.plasma.networkmanagement", "org.kde.muonnotifier", "org.kde.ktp-contactlist", "org.kde.plasma.clipboard", "org.kde.plasma.notifications", "org.kde.plasma.battery", "org.kde.plasma.printmanager", "org.kde.plasma.devicenotifier", "org.kde.plasma.bluetooth", "org.kde.plasma.mediacontroller")) ,  QVariant(QString, "org.kde.phone.notifications") ) )

so, like javascript thinks plasmoid.configuration.knownItems is not an array, doing a concat over this creates an array of two items, the first the full content it used to have, the second the new values.

It also suggests your patch to ConfigPropertyMapPrivate is correct, there is just also another problem on top of that one ;)


- Marco


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123789/#review80346
-----------------------------------------------------------


On May 14, 2015, 11:20 a.m., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123789/
> -----------------------------------------------------------
> 
> (Updated May 14, 2015, 11:20 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 345872
>     https://bugs.kde.org/show_bug.cgi?id=345872
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> if new plasmoids are installed that are systray enabled and enabled by default, automatically add them, unless they were disabled explicitly. This solves a problem distributions seem to add while updating to new plasma releases.
> 
> obtain this by keeping both a list of explicitly enabled stuff and all the already known stuff
> 
> it does horrible things with arrays, that's due to some weird sideeffects of having qstringlists as properties, of which we'll have to pay close attention to
> 
> 
> Diffs
> -----
> 
>   applets/systemtray/package/contents/config/main.xml 2967dba 
>   applets/systemtray/package/contents/ui/main.qml fd59a14 
> 
> Diff: https://git.reviewboard.kde.org/r/123789/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150514/bed7ecc6/attachment.html>