----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128580/#review98035 -----------------------------------------------------------
Why does it break for SNIs? It first tries QIcon::fromTheme and if it fails (isNull()) fallbacks to original QIcon. This patch just switched it, why does it make a difference? The problem I see here is that if a custom icon loader is used which uses static icon name (let's say "plasma" as we have it in breeze icons), but the actual plasma.png file representing completely different icon. In this case, however, it would fail in the codepath above - being loaded either from plasma theme or svg with kiconloader. - David Rosca On Aug. 2, 2016, 9:11 p.m., David Edmundson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128580/ > ----------------------------------------------------------- > > (Updated Aug. 2, 2016, 9:11 p.m.) > > > Review request for KDE Frameworks and Plasma. > > > Repository: plasma-framework > > > Description > ------- > > Currently the code gets the icon name from the QIcon and tries to do > some Plasma theming with it. > However if that fails it then loads the QIcon::fromTheme again. > > This is pointless in most cases and will break any icons that have a > custom loader (all SNIs) > > > Diffs > ----- > > src/declarativeimports/core/iconitem.cpp > 29c7f05b5df060df7b362b331f7edc412df12307 > > Diff: https://git.reviewboard.kde.org/r/128580/diff/ > > > Testing > ------- > > > Thanks, > > David Edmundson > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
