----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118561/#review59358 -----------------------------------------------------------
Thanks Martin for looking at the issue. Just one question: Is plasmashell the only application which needs to include the additional search path, or does every application wanting a tray icon call the addAppDir() function? I am bit worried about any additional "stat" calls. Those we have already slow down the icon lookup considerably. src/kiconloader.h <https://git.reviewboard.kde.org/r/118561/#comment41309> Please write "directory" instead of "dir" (which is also the abbreviation for "direction"). src/kiconloader.h <https://git.reviewboard.kde.org/r/118561/#comment41308> Are we still allowed to break binary compatibility? If not, please create a separate call. src/kicontheme.h <https://git.reviewboard.kde.org/r/118561/#comment41310> The comment does not clarify where the additional hint is included in the search chain (first or last). - Christoph Feck On June 5, 2014, 10:52 a.m., Martin Klapetek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/118561/ > ----------------------------------------------------------- > > (Updated June 5, 2014, 10:52 a.m.) > > > Review request for KDE Frameworks and Christoph Feck. > > > Repository: kiconthemes > > > Description > ------- > > KIconTheme in kdelibs4 was searching KGlobal::dirs()->resourceDirs("data") to > find all icons; the status notifier dataengine was then setting custom > resourceDirs(..) with custom SNI theme paths (which the SNIs can pass) and so > the SNI icons always ended up in the theme search paths (the SNI icons are > stored as a whole theme). > > With the port to QStandardPaths, we lost the ability to pass custom dirs into > the theme search paths and that results in status notifier icons in Plasma > Next having "unknown" icons as their icon theme paths are not searched and so > icons are not found. This is the case mostly of the Qt4 apps running on Qt > with the QSystrayIcon-to-SNI-patches (case of *buntu and I heard opensuse > too?) > > KIconLoader however has "addAppDir(..)" method, so I expanded that method to > actually take an "app dir" and add it to the theme search paths. Plasma Next > now have proper icons in the systray. > > > Diffs > ----- > > src/kiconloader.h 68ccd09 > src/kiconloader.cpp 4080a1d > src/kicontheme.h 73011e2 > src/kicontheme.cpp 12337e8 > > Diff: https://git.reviewboard.kde.org/r/118561/diff/ > > > Testing > ------- > > > Thanks, > > Martin Klapetek > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
