IconItem: Don't overwrite source when using QIcon::name()

source should always return the same value as it was assigned.
Also removed duplicated code (empty sourceString is now handled
as if the source couldn't be converted to any type).

REVIEW: 127282
This commit is contained in:
David Rosca 2016-03-04 23:44:57 +01:00
parent da340aaefa
commit 978b8b3b87
4 changed files with 48 additions and 24 deletions

View File

@ -76,6 +76,11 @@ void IconItemTest::initTestCase()
}
}
void IconItemTest::cleanupTestCase()
{
delete m_view;
}
void IconItemTest::cleanup()
{
qDeleteAll(m_view->rootObject()->childItems());
@ -138,6 +143,7 @@ void IconItemTest::loadPixmap()
QImage capture = grabImage(item.data());
QCOMPARE(capture, sourcePixmap.toImage().convertToFormat(QImage::Format_ARGB32_Premultiplied));
QCOMPARE(sourcePixmap, item->property("source").value<QPixmap>());
}
//tests setting icon from a QImage
@ -151,9 +157,9 @@ void IconItemTest::loadImage()
QImage capture = grabImage(item.data());
QCOMPARE(capture, sourceImage.convertToFormat(QImage::Format_ARGB32_Premultiplied));
QCOMPARE(sourceImage, item->property("source").value<QImage>());
}
void IconItemTest::invalidIcon()
{
QString name("tst-plasma-framework-invalid-icon-name");
@ -179,6 +185,7 @@ void IconItemTest::usesPlasmaTheme()
QQuickItem *item1 = createIconItem();
item1->setProperty("source", "konversation");
QVERIFY(item1->property("valid").toBool());
QCOMPARE(QStringLiteral("konversation"), item1->property("source").toString());
Plasma::Svg svg;
svg.setContainsMultipleImages(true);
@ -266,6 +273,7 @@ void IconItemTest::bug_359388()
QQuickItem *item1 = createIconItem();
item1->setProperty("source", customThemeIcon);
QVERIFY(item1->property("valid").toBool());
QCOMPARE(customThemeIcon, item1->property("source").value<QIcon>());
QQuickItem *item2 = createIconItem();
item2->setProperty("source", QIcon(QFINDTESTDATA("data/bug359388/hicolor/22x22/apps/" + name + ".svg")));
@ -325,5 +333,24 @@ void IconItemTest::themeChange()
QVERIFY(img1 != img2);
}
void IconItemTest::qiconFromTheme()
{
// Icon from Plasma theme
QQuickItem *item1 = createIconItem();
item1->setProperty("source", QIcon::fromTheme("zoom-fit-height"));
QIcon icon1 = QIcon::fromTheme("zoom-fit-height");
QVERIFY(item1->findChild<Plasma::Svg*>());
QVERIFY(!imageIsEmpty(grabImage(item1)));
QCOMPARE(icon1, item1->property("source").value<QIcon>());
// Icon from icon theme
QQuickItem *item2 = createIconItem();
QIcon icon2 = QIcon::fromTheme("tst-plasma-framework-test-icon");
item2->setProperty("source", icon2);
QVERIFY(item2->findChild<Plasma::Svg*>());
QVERIFY(!imageIsEmpty(grabImage(item2)));
QCOMPARE(icon2, item2->property("source").value<QIcon>());
}
QTEST_MAIN(IconItemTest)

View File

@ -35,6 +35,7 @@ class IconItemTest : public QObject
private Q_SLOTS:
void initTestCase();
void cleanupTestCase();
void cleanup();
void loadPixmap();
@ -47,6 +48,7 @@ private Q_SLOTS:
void bug_359388();
void loadSvg();
void themeChange();
void qiconFromTheme();
private:
QQuickItem *createIconItem();

View File

@ -46,7 +46,6 @@ IconItem::IconItem(QQuickItem *parent)
m_usesPlasmaTheme(true),
m_textureChanged(false),
m_sizeChanged(false),
m_svgFromIconLoader(false),
m_colorGroup(Plasma::Theme::NormalColorGroup),
m_animValue(0)
{
@ -85,28 +84,20 @@ void IconItem::setSource(const QVariant &source)
}
m_source = source;
QString sourceString = source.toString();
// If the QIcon was created with QIcon::fromTheme(), try to load it as svg
if (m_source.canConvert<QIcon>() && !m_source.value<QIcon>().name().isEmpty()) {
m_source = m_source.value<QIcon>().name();
if (source.canConvert<QIcon>() && !source.value<QIcon>().name().isEmpty()) {
sourceString = source.value<QIcon>().name();
}
if (m_source.canConvert<QString>()) {
const QString sourceString = m_source.toString();
if (sourceString.isEmpty()) {
delete m_svgIcon;
m_svgIcon = 0;
m_icon = QIcon();
emit validChanged();
schedulePixmapUpdate();
return;
}
if (!sourceString.isEmpty()) {
//If a url in the form file:// is passed, take the image pointed by that from disk
QUrl url(sourceString);
if (url.isLocalFile()) {
m_icon = QIcon();
m_imageIcon = QImage(url.path());
m_svgIconName.clear();
delete m_svgIcon;
m_svgIcon = 0;
} else {
@ -126,6 +117,7 @@ void IconItem::setSource(const QVariant &source)
//success?
if (m_svgIcon->isValid() && m_svgIcon->hasElement(sourceString)) {
m_icon = QIcon();
m_svgIconName = sourceString;
//ok, svg not available from the plasma theme
} else {
@ -140,10 +132,10 @@ void IconItem::setSource(const QVariant &source)
} else {
qWarning() << "KIconLoader has no theme set";
}
m_svgFromIconLoader = !iconPath.isEmpty();
if (m_svgFromIconLoader) {
if (!iconPath.isEmpty()) {
m_svgIcon->setImagePath(iconPath);
m_svgIconName = sourceString;
//fail, use QIcon
} else {
m_icon = QIcon::fromTheme(sourceString);
@ -151,6 +143,7 @@ void IconItem::setSource(const QVariant &source)
// fallback for non-theme icons
m_icon = source.value<QIcon>();
}
m_svgIconName.clear();
delete m_svgIcon;
m_svgIcon = 0;
m_imageIcon = QImage();
@ -161,17 +154,20 @@ void IconItem::setSource(const QVariant &source)
} else if (source.canConvert<QIcon>()) {
m_icon = source.value<QIcon>();
m_imageIcon = QImage();
m_svgIconName.clear();
delete m_svgIcon;
m_svgIcon = 0;
} else if (source.canConvert<QImage>()) {
m_icon = QIcon();
m_imageIcon = source.value<QImage>();
m_svgIconName.clear();
delete m_svgIcon;
m_svgIcon = 0;
} else {
m_icon = QIcon();
m_imageIcon = QImage();
m_svgIconName.clear();
delete m_svgIcon;
m_svgIcon = 0;
}
@ -393,15 +389,15 @@ void IconItem::loadPixmap()
return;
} else if (m_svgIcon) {
m_svgIcon->resize(size, size);
if (m_svgIcon->hasElement(m_source.toString())) {
result = m_svgIcon->pixmap(m_source.toString());
} else if (m_svgFromIconLoader) {
if (m_svgIcon->hasElement(m_svgIconName)) {
result = m_svgIcon->pixmap(m_svgIconName);
} else if (!m_svgIconName.isEmpty()) {
const auto *iconTheme = KIconLoader::global()->theme();
QString iconPath;
if (iconTheme) {
iconPath = iconTheme->iconPath(source().toString() + QLatin1String(".svg"), qMin(width(), height()), KIconLoader::MatchBest);
iconPath = iconTheme->iconPath(m_svgIconName + QLatin1String(".svg"), qMin(width(), height()), KIconLoader::MatchBest);
if (iconPath.isEmpty()) {
iconPath = iconTheme->iconPath(source().toString() + QLatin1String(".svgz"), qMin(width(), height()), KIconLoader::MatchBest);
iconPath = iconTheme->iconPath(m_svgIconName + QLatin1String(".svgz"), qMin(width(), height()), KIconLoader::MatchBest);
}
} else {
qWarning() << "KIconLoader has no theme set";

View File

@ -155,6 +155,7 @@ private:
//all the ways we can set an source. Only one of them will be valid
QIcon m_icon;
Plasma::Svg *m_svgIcon;
QString m_svgIconName;
QPixmap m_pixmapIcon;
QImage m_imageIcon;
//this contains the raw variant it was passed
@ -170,8 +171,6 @@ private:
bool m_textureChanged;
bool m_sizeChanged;
bool m_svgFromIconLoader;
QPixmap m_iconPixmap;
QPixmap m_oldIconPixmap;