KDE/kdebase/apps/plasma/applets/folderview

[Fredrik Höglund]

On Sunday 12 April 2009, Aaron J. Seigo wrote:
> On Thursday 09 April 2009, Fredrik Höglund wrote:
> > SVN commit 951572 by fredrik:
> >
> > Include actions in the drop menu for creating applets for the dropped
> > URL's, and for setting the wallpaper.
> 
> this is broken in so many ways:
> 
> * it assumes there's an Image plugin on the system
> 
> * it assumes Image has a mode called SingleImage

Which are all pretty safe assumptions to make since kdebase installs
that very plugin, and folderview is in kdebase.

Plasma::Containment, which is in kdelibs, makes the same assumptions
even though installing kdebase isn't a requirement for running an
application that uses libplasma.

> * it assumes that the dropped file should actually be used with the Image 
> plugin, when really if i drop an image and i'm using the Virus paper (for 
> instance) it should change the image file used by Virus without switching to 
> Image

Switching to the image plugin seemed like the only reasonable thing to
do, since there is no way of knowing if the current plugin is suitable for
or even capable of displaying the dropped image.

> * it creates a behaviour that is only found in one containment when really 
> this should probably be found in all Containments where drawWallpaper()

This sentence is incomplete, but if what you're saying is that this
feature should be supported by all containments, I agree.

I was planning on asking you if you wanted the related functions
moved to libplasma for that reason.
 
> * it prevents drops of files creating widgets such as the Frame widget (though 
> in this case, that may be warranted / desired given that it's a "compatibility 
> mode" containment?)

If you had read the code, the commit message you're replying to,
or actually tried it, you would have seen that it adds the
"Set as Wallpaper" option in addition to the options for creating
widgets.

> * it contains assumptions about how the configuration is stored for the Image 
> plugin; changes in the Image plugin or how Containment deals with wallpaper 
> config data can and will break this.

Indeed it does, and I'm not particularly happy about that. But after
studying the wallpaper related classes in Plasma I concluded that this
was the only way it could be done with the current code base.

In the unlikely event that someone decides to make gratuitous changes
to the way the configuration is stored before 4.3, with all the work that
involves of writing migration scripts and what not, I'll update the code
of course.

But I don't have the time to change the wallpaper classes to make it
possible to tell the plugins to change the displayed image or query
them about their capabilities before 4.3. 

> * does it actually do this when it's a widget and not a containment?

Again, if you actually read the code you'll see that it doesn't.
 
> the way this ought to be done is to have Wallpapers register supported 
> mimetypes in their .desktop file and query these on file drops and merge those 
> possibilities with the menu created in the KUrl::List::canDecode(event-
> >mimeData()) branch in Containment::dropEvent

KMimeType doesn't resolve mimetypes for http:// URL's, and one of
the most common use cases I see for this feature is dragging an image
from a web browser window and dropping it on the desktop.

I don't think it's reasonable to download the file first, determine the
mimetype, and only then show the menu.

I'm also not sure I like the idea of making the user choose a wallpaper
plugin when they drop an image, since it requires them to understand
what the plugin does based on the name.

I imagine few users would click a menu entry labeled "Virus", after
dropping something from a web browser on their desktop.

I think it would be better to try to use the current plugin if it claims
it can display the image, or switch to the default plugin otherwise.

> please revert this particular commit and let's do it properly.

Like I said above, I don't have the time to make any changes to the
way the wallpaper plugins work, so if you find this feature that
objectionable I will just disable it.

Regards,
Fredrik