D29668: Do not reject icon theme dir with invalid context/type.

[Xuetian Weng]

xuetianweng added a comment.


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

REPOSITORY
  R302 KIconThemes

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

To: xuetianweng, #frameworks, dfaure, apol
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D29668: Do not reject icon theme dir with invalid context/type.

[Xuetian Weng]

xuetianweng added a comment.


  In D29668#669069 , @dfaure wrote:
  
  > 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

REPOSITORY
  R302 KIconThemes

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

To: xuetianweng, #frameworks, dfaure, apol
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D29668: Do not reject icon theme dir with invalid context/type.

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kicontheme.cpp:761
>  } else if (tmp.isEmpty()) {
>  // do nothing. key not required
>  } else {

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

> kicontheme.cpp:774
>  qWarning() << "Invalid Type=" << tmp << "line for icon theme: " << 
> constructFileName(QString());
> -return;
> +mType = KIconLoader::Threshold;
>  }

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.

REPOSITORY
  R302 KIconThemes

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

To: xuetianweng, #frameworks, dfaure, apol
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D29668: Do not reject icon theme dir with invalid context/type.

[Xuetian Weng]

xuetianweng created this revision.
xuetianweng added a reviewer: Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
xuetianweng requested review of this revision.

REVISION 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.

REPOSITORY
  R302 KIconThemes

BRANCH
  master

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

AFFECTED FILES
  src/kicontheme.cpp

To: xuetianweng, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29668: Do not reject icon theme dir with invalid context/type.

[Xuetian Weng]

xuetianweng edited the test plan for this revision.

REPOSITORY
  R302 KIconThemes

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

To: xuetianweng, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29668: Do not reject icon theme dir with invalid context/type.

[Xuetian Weng]

xuetianweng added reviewers: dfaure, apol.

REPOSITORY
  R302 KIconThemes

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

To: xuetianweng, #frameworks, dfaure, apol
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29668: Do not reject icon theme dir with invalid context/type.

[Xuetian Weng]

xuetianweng edited the test plan for this revision.

REPOSITORY
  R302 KIconThemes

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

To: xuetianweng, #frameworks, dfaure, apol
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29668: Do not reject icon theme dir with invalid context/type.

[David Edmundson]

davidedmundson added a comment.


  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!"

REPOSITORY
  R302 KIconThemes

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

To: xuetianweng, #frameworks, dfaure, apol
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D29668: Do not reject icon theme dir with invalid context/type.

[David Faure]

dfaure added a comment.


  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/

REPOSITORY
  R302 KIconThemes

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

To: xuetianweng, #frameworks, dfaure, apol
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns