Review Request: Do not check if a wallpaper is a valid Plasma::Package

[Aaron J. Seigo]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101302/#review3150
-----------------------------------------------------------


this is the wrong fix. well, it's not really a fix at all, it just allows any broken thing to go through. removing error checking is rarely the answer ;) clearly, it is failing for single image file wallpapers, which means Package::isValid() is thinking the package is missing things.

if you apply this simple patch in kdelibs/plasma, does it fix things for you:

diff --git a/plasma/private/packages.cpp b/plasma/private/packages.cpp
index 5e064d9..f24a283 100644
--- a/plasma/private/packages.cpp
+++ b/plasma/private/packages.cpp
@@ -266,6 +266,7 @@ void WallpaperPackage::pathChanged()
         findBestPaper();
     } else {
         // dirty trick to support having a file passed in instead of a directory
+        removeDefinition("images");
         addFileDefinition("preferred", info.fileName(), i18n("Recommended wallpaper file"));
         setContentsPrefixPaths(QStringList());
         //kDebug() << "changing" << path() << "to" << info.path();


- Aaron J.


On May 6, 2011, 5:17 a.m., Raphael Kubo da Costa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101302/
> -----------------------------------------------------------
> 
> (Updated May 6, 2011, 5:17 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> Do not check if the created Plasma::Package is valid.
>     
> Wallpapers consisting of a single image (without a .desktop metadata or anything else), usually obtained via GHNS, are not valid packages, but are valid wallpapers.
>     
> Commit b60136e, besides moving the loading code to a separate thread, also started checking if a given found path was also a valid package, which kept any wallpaper downloaded via the "get new wallpapers" button from being listed at all.
> 
> 
> This addresses bug 269587.
>     http://bugs.kde.org/show_bug.cgi?id=269587
> 
> 
> Diffs
> -----
> 
>   plasma/generic/wallpapers/image/backgroundlistmodel.cpp 3c92024 
> 
> Diff: http://git.reviewboard.kde.org/r/101302/diff
> 
> 
> Testing
> -------
> 
> All downloaded wallpapers are listed and selectable again.
> 
> 
> Thanks,
> 
> Raphael
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20110506/a7ee69ea/attachment-0001.htm