D16742: Fix iconitemtest regression

2018-11-08 Thread José Manuel Santamaría Lema
joselema closed this revision.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D16742

To: joselema, mart, apol
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D16742: Fix iconitemtest regression

2018-11-08 Thread José Manuel Santamaría Lema
joselema added a commit: R242:ed1724c63e9f: Summary: After this commit 
https://cgit.kde.org/plasma-framework.git/commit/?.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D16742

To: joselema, mart, apol
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D16742: Fix iconitemtest regression

2018-11-07 Thread Marco Martin
mart accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D16742

To: joselema, mart, apol
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D16742: Fix iconitemtest regression

2018-11-07 Thread José Manuel Santamaría Lema
joselema created this revision.
joselema added reviewers: mart, apol.
Herald added a project: Frameworks.
Herald removed a subscriber: Frameworks.
joselema requested review of this revision.

REVISION SUMMARY
  Hi,
  
  after this commit
  
https://cgit.kde.org/plasma-framework.git/commit/?id=9f08668147d2e99f8b5069ff50d5c54e672a87af
  
  The iconitemtest started to fail. As we can see from that commit the set of 
changes starts with this:
  
if (sourceString.endsWith(QLatin1String(".svg")) ||
sourceString.endsWith(QLatin1String(".svgz"))) {
m_icon = QIcon(localFile);
m_imageIcon = QImage();
} else {
m_icon = QIcon(localFile);
m_imageIcon = QImage();
}
  
  As we can see the code executed in the "if" block and the "else" block is 
exactly the same. My guess (and it's just a wild guess because I don't actually 
understand the code involved) is that you actually wanted to do this:
  
if (sourceString.endsWith(QLatin1String(".svg")) ||
sourceString.endsWith(QLatin1String(".svgz"))) {
m_icon = QIcon(localFile);
m_imageIcon = QImage();
} else {
m_icon = QIcon();
m_imageIcon = QImage(localfile);
}
  
  So I'm attaching a patch doing that ↑

TEST PLAN
  I have just built the code with the patch, it fixes the test for me, but I'm 
not sure if the patch is correct.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D16742

AFFECTED FILES
  src/declarativeimports/core/iconitem.cpp

To: joselema, mart, apol
Cc: kde-frameworks-devel, michaelh, ngraham, bruns