Do not reject icon theme dir with invalid context/type.
Needs RevisionPublic

Authored by xuetianweng on Mon, May 11, 8:50 PM.

Details

Reviewers
dfaure
apol
Group Reviewers
Frameworks
Summary

For example, gnome's default theme adwaita has their own context type like
Legacy/UI. This check prevents the icon with such lines to be rendered for Qt app
running under KDE. Even if it would result in some unexpected result, it's still
better than not rendering the icon.

Test Plan

Test with gnome default icon theme.

vs

Diff Detail

Repository
R302 KIconThemes
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 26747
Build 26765: arc lint + arc unit
xuetianweng created this revision.Mon, May 11, 8:50 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptMon, May 11, 8:50 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
xuetianweng requested review of this revision.Mon, May 11, 8:50 PM
xuetianweng edited the test plan for this revision. (Show Details)Mon, May 11, 8:51 PM
xuetianweng added reviewers: dfaure, apol.
xuetianweng edited the test plan for this revision. (Show Details)

Seems sensible, given it's valid to have an empty context.

I don't know enough about icons to know all possible ill effects. If there's no other comments in a few days consider this a "ship it!"

What are the values of Context and Type?

"Legacy" and "UI" ?

I can't see anything in index.theme https://ftp.gnome.org/pub/GNOME/sources/adwaita-icon-theme/3.36/

What are the values of Context and Type?

"Legacy" and "UI" ?

I can't see anything in index.theme https://ftp.gnome.org/pub/GNOME/sources/adwaita-icon-theme/3.36/

Yes, legacy and ui.

[8x8/legacy]
Context=Legacy
Size=8
Type=Fixed

You can try to install one from your distro to check the actual content. index.theme is generated during the "configure & make".

dfaure requested changes to this revision.Tue, May 26, 11:02 PM
dfaure added inline comments.
src/kicontheme.cpp
761

(OK, if this key is not required then surely unknown values are fine too..)

774

So, if index.theme says Type=Fixed, why did you change this line? It should work without this second change, only the first one seems useful.

This revision now requires changes to proceed.Tue, May 26, 11:02 PM