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

[Raphael Kubo da Costa]

> On May 6, 2011, 7:28 a.m., Aaron J. Seigo wrote:
> > 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. Seigo wrote:
>     ok, i just committed a slightly better fix than the above path to master to kdelibs/plasma/private/packages.cp. can you check if that works for you and if so i'll backport it.

Yep, it works fine now, thanks.

I'm closing the review as discarded now.


- Raphael


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


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/f767f299/attachment.htm