--===============8906387994794610353== Content-Type: multipart/alternative; boundary="===============1108594164342934963==" --===============1108594164342934963== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128397/#review97193 ----------------------------------------------------------- src/kiconengine.cpp (line 164) Ideally, one would want that even if compiled with older Qt, this works as Qt is upgraded. The way to do that would be to hardcode the numerical value of QIconEngine::IsNullHook when compiling with older Qt. - Olivier Goffart On July 7, 2016, 9:55 p.m., David Rosca wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128397/ > ----------------------------------------------------------- > > (Updated July 7, 2016, 9:55 p.m.) > > > Review request for KDE Frameworks and Olivier Goffart. > > > Repository: kiconthemes > > > Description > ------- > > QIcon::hasThemeIcon(name) checks if QIcon::name() == name, so icon engine must return empty string when icon doesn't exist. > Also implement IsNullHook for Qt 5.7. Comes with autotest. > > > Diffs > ----- > > autotests/CMakeLists.txt 0c7de50 > autotests/kiconengine_unittest.cpp PRE-CREATION > src/kiconengine.h 21a63f5 > src/kiconengine.cpp 3ccc7d1 > > Diff: https://git.reviewboard.kde.org/r/128397/diff/ > > > Testing > ------- > > Issue was reported in https://bugreports.qt.io/browse/QTBUG-54595 > Also similar issue https://bugs.kde.org/show_bug.cgi?id=365031 (there is a check for hasThemeIcon before using the icon, but it returns true which results in invalid icon name -> no icon). > > > Thanks, > > David Rosca > > --===============1108594164342934963== MIME-Version: 1.0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 7bit

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

src/kiconengine.cpp (Diff revision 1)
		164	#if QT_VERSION >= QT_VERSION_CHECK(5, 7, 0)

Ideally, one would want that even if compiled with older Qt, this works as Qt is upgraded.

The way to do that would be to hardcode the numerical value of QIconEngine::IsNullHook when compiling with older Qt.

- Olivier Goffart

On July 7th, 2016, 9:55 p.m. UTC, David Rosca wrote:

Review request for KDE Frameworks and Olivier Goffart.

By David Rosca.

Updated July 7, 2016, 9:55 p.m.

Repository: kiconthemes

Description

QIcon::hasThemeIcon(name) checks if QIcon::name() == name, so icon engine must return empty string when icon doesn't exist.
Also implement IsNullHook for Qt 5.7. Comes with autotest.

Testing

Issue was reported in https://bugreports.qt.io/browse/QTBUG-54595 Also similar issue https://bugs.kde.org/show_bug.cgi?id=365031 (there is a check for hasThemeIcon before using the icon, but it returns true which results in invalid icon name -> no icon).

Diffs

autotests/CMakeLists.txt (0c7de50)
autotests/kiconengine_unittest.cpp (PRE-CREATION)
src/kiconengine.h (21a63f5)
src/kiconengine.cpp (3ccc7d1)

--===============1108594164342934963==-- --===============8906387994794610353== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KS2RlLWZyYW1l d29ya3MtZGV2ZWwgbWFpbGluZyBsaXN0CktkZS1mcmFtZXdvcmtzLWRldmVsQGtkZS5vcmcKaHR0 cHM6Ly9tYWlsLmtkZS5vcmcvbWFpbG1hbi9saXN0aW5mby9rZGUtZnJhbWV3b3Jrcy1kZXZlbAo= --===============8906387994794610353==--