[Icon Item] Treat sources starting with a slash as local file
ClosedPublic

Authored by broulik on Jan 11 2018, 9:40 AM.

Details

Summary

We have a special case for sources starting with "file://" but a "/" also represents an absolute path and shouldn't conflict with icon theme names.
Kicker sets a custom image as local path and then we would end up trying to load it as a QIcon::fromTheme eventually.
This will cause the implicit size of the icon item to stay at its default as we only check a custom implicit size for a source QImage or an SVG. Moreover, this potentially introduces scaling artefacts.

Test Plan

Unit test still passes. Comes with a new unit test to verify that non-square images get their proper implicit size set both when loaded as URL and local path. Fails before, passes with this fix.

Kicker showing proper non-square icon again

Diff Detail

Repository
R242 Plasma Framework (Library)
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik created this revision.Jan 11 2018, 9:40 AM
Restricted Application added projects: Plasma, Frameworks. · View Herald TranscriptJan 11 2018, 9:40 AM
Restricted Application added subscribers: Frameworks, plasma-devel. · View Herald Transcript
mwolff requested changes to this revision.Jan 11 2018, 10:00 AM
mwolff added a subscriber: mwolff.

lgtm in general, but can be cleaned up

src/declarativeimports/core/iconitem.cpp
154

make this a QString localFile;

156–161

assign localFile = QUrl(sourceString).toLocalFile(); here

158

assign localFile = sourceString; here

163

use localFile here

This revision now requires changes to proceed.Jan 11 2018, 10:00 AM
broulik updated this revision to Diff 25144.Jan 11 2018, 10:03 AM
  • Incorporate suggestions by milian
mwolff accepted this revision.Jan 11 2018, 10:24 AM

lgtm

This revision is now accepted and ready to land.Jan 11 2018, 10:24 AM
davidedmundson accepted this revision.Jan 15 2018, 9:12 AM
mart accepted this revision.Jan 15 2018, 9:16 AM
This revision was automatically updated to reflect the committed changes.