D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

This revision was automatically updated to reflect the committed changes. Closed by commit R241:eb20176d1a42: KIO::iconNameForUrl: fix searching for kde protocol icons (authored by meven). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27539?vs=76156=76162

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH arcpatch-D27539 REVISION DETAIL https://phabricator.kde.org/D27539 To: meven, ngraham, #frameworks, dfaure, broulik, sitter Cc: sitter, kde-frameworks-devel, LeGast00n, cblack,

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

meven updated this revision to Diff 76156. meven marked 2 inline comments as done. meven added a comment. Improve comment, remove unnecessary check REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27539?vs=76148=76156 BRANCH arcpatch-D27539 REVISION DETAIL

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > global.cpp:240 > // Let KFileItem::iconName handle things for us > -if (i == unknown || i.isEmpty() || mt.isDefault()) { > +if

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

meven added a comment. In D27539#615654 , @dfaure wrote: > This is about an icon name. Apps don't (shouldn't) "check the value". > > We should return application-octet-stream if we did find the file, but mimetype determination failed. That's

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

meven updated this revision to Diff 76148. meven added a comment. Add an https test case REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27539?vs=76147=76148 BRANCH arcpatch-D27539 REVISION DETAIL https://phabricator.kde.org/D27539 AFFECTED FILES

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

dfaure added a comment. This is about an icon name. Apps don't (shouldn't) "check the value". We should return application-octet-stream if we did find the file, but mimetype determination failed. That's what this mimetype and its icon are about. We should return unknown if we have

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

meven added a comment. In D27539#615288 , @dfaure wrote: > kfileitemtest still passes? It does, and I added more tests. A question I have is that in case we don't find an icon depending on how we determine it we can return

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

meven updated this revision to Diff 76147. meven added a comment. Add some tests REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27539?vs=76106=76147 BRANCH arcpatch-D27539 REVISION DETAIL https://phabricator.kde.org/D27539 AFFECTED FILES

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

dfaure added a comment. kfileitemtest still passes? REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D27539 To: meven, ngraham, #frameworks, dfaure, broulik, sitter Cc: sitter, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

ngraham accepted this revision. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D27539 To: meven, ngraham, #frameworks, dfaure, broulik, sitter Cc: sitter, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

meven marked an inline comment as done. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D27539 To: meven, ngraham, #frameworks, dfaure, broulik, sitter Cc: sitter, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

meven updated this revision to Diff 76106. meven added a comment. use QStringLitteral REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27539?vs=76088=76106 BRANCH master REVISION DETAIL https://phabricator.kde.org/D27539 AFFECTED FILES

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

sitter accepted this revision. sitter added a comment. This revision is now accepted and ready to land. I've changed 417069 to CCBUG only, there's more to that issue than just the bogus fallback in this function. Diff LGTM, besides one minor comment about not keeping that QLatin1String.

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

sitter edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D27539 To: meven, ngraham, #frameworks, dfaure, broulik Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

meven edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D27539 To: meven, ngraham, #frameworks, dfaure, broulik Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

meven added reviewers: dfaure, broulik. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D27539 To: meven, ngraham, #frameworks, dfaure, broulik Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

meven updated this revision to Diff 76088. meven added a comment. Clean code further REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27539?vs=76087=76088 BRANCH master REVISION DETAIL https://phabricator.kde.org/D27539 AFFECTED FILES

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

meven edited the summary of this revision. meven edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D27539 To: meven, ngraham, #frameworks Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

meven updated this revision to Diff 76087. meven added a comment. Simplify REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27539?vs=76086=76087 BRANCH master REVISION DETAIL https://phabricator.kde.org/D27539 AFFECTED FILES src/core/global.cpp To:

D27539: KIO::iconNameForUrl: fix searching for kde protocol icons

meven created this revision. meven added reviewers: ngraham, Frameworks. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. meven requested review of this revision. REVISION SUMMARY BUG: 417922 BUG: 417921 BUG: 417069 FIXED-IN: 5.68 REPOSITORY R241