Bug 327580 - [PATCH] Wallpaper -> Slideshow -> Custom Folders not persisted in the menu
Summary: [PATCH] Wallpaper -> Slideshow -> Custom Folders not persisted in the menu
Status: RESOLVED FIXED
Alias: None
Product: plasma4
Classification: Unmaintained
Component: wallpaper-image (show other bugs)
Version: 4.11.5
Platform: unspecified Linux
: HI minor
Target Milestone: ---
Assignee: Paolo Capriotti
URL:
Keywords: regression
: 323030 325976 328843 329147 330130 331923 334926 335115 337619 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-11-13 23:10 UTC by Mircea Kitsune
Modified: 2015-01-03 00:09 UTC (History)
21 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.11.12
Sentry Crash Report:


Attachments
settings that are present after clicking OK and opening the dialog again (73.04 KB, image/png)
2013-11-21 08:08 UTC, Alexander Onic
Details
diff that fix this BUG. (646 bytes, application/octet-stream)
2014-02-23 11:09 UTC, TOM Harrison
Details
According to the comments, this solves the problem (646 bytes, patch)
2014-02-27 20:09 UTC, Alexander Blinne
Details
patch maybe in a better format (976 bytes, patch)
2014-05-16 17:16 UTC, François Valenduc
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mircea Kitsune 2013-11-13 23:10:52 UTC
The "Custom folders" section of the Slideshow wallpaper type is not persisted when re-opening the menu.

Reproducible: Always

Steps to Reproduce:
1. Right click your desktop and select "Desktop Settings". Make sure you are in the "View" tab.
2. Set "Wallpaper" to Slideshow, disable "System wallpapers" and "My downloaded wallpapers", then add one or more custom folders. Click OK to exit.
3. Repeat step 1 and go back into the "Desktop Settings" window.
Actual Results:  
The "Custom folders" list is now empty.

Expected Results:  
It should be loaded when the menu opens.

This only affects the menu, actual configuration is stored properly and the slideshow works. Clicking OK with the empty list maintains the same folders, and new folders added to the list are properly applied. My previous KDE version was 4.10.5 in which this did not happen.
Comment 1 Alexander Onic 2013-11-21 08:06:27 UTC
I have the same problem in Kubuntu's current KDE 4.11.2.

In the View category, the wallpaper is set to "Slideshow". The settings I make in the Images section are applied, when clicking "Apply" (different and correct wallpaper is shown), but immediately forgotten and reverted when clicking OK (one of the system standard wallpapers is shown).
The selections affected are "System wallpapers", "My downloaded wallpapers" and "Custom folders".
Other settings, like "Change images every" setting seem to work properly.
Comment 2 Alexander Onic 2013-11-21 08:08:53 UTC
Created attachment 83673 [details]
settings that are present after clicking OK and opening the dialog again

The settings in the "Images" part are completely lost after clicking OK. The picture shows the settings after opening the settings dialog again.
Comment 3 Alexander Onic 2013-12-03 10:57:53 UTC
Could there please be any sign not emphasizing the total insignificance of our bug report?
Be it the wish for further information or a patch?
Comment 4 Christoph Feck 2013-12-15 19:47:02 UTC
*** Bug 325976 has been marked as a duplicate of this bug. ***
Comment 5 Christoph Feck 2013-12-15 19:48:13 UTC
*** Bug 328843 has been marked as a duplicate of this bug. ***
Comment 6 Christoph Feck 2013-12-15 19:50:21 UTC
*** Bug 323030 has been marked as a duplicate of this bug. ***
Comment 7 TOM Harrison 2013-12-16 08:36:18 UTC
any progress about this bug?
Comment 8 Mircea Kitsune 2013-12-16 11:45:06 UTC
Still there in 4.11.3.
Comment 9 Christoph Feck 2013-12-23 15:19:29 UTC
*** Bug 329147 has been marked as a duplicate of this bug. ***
Comment 10 hitori.gm 2014-01-17 17:05:56 UTC
Still persists in 4.12.1 (Arch Linux)
Comment 11 Christoph Feck 2014-01-18 17:04:54 UTC
*** Bug 330130 has been marked as a duplicate of this bug. ***
Comment 12 Mark C 2014-01-18 17:41:48 UTC
I'm running KDE 4.12.1 on Arch. This problem still persists.
Comment 13 oberonking76 2014-01-21 16:02:26 UTC
Same Here... KDE 4.11.5 on Fedora 20
Comment 14 Maximiliano Curia 2014-02-10 21:33:02 UTC
Hi,

These bug was also reported by Debian users here:
http://bugs.debian.org/738337

Thanks,
Comment 15 sparhawk 2014-02-16 03:41:50 UTC
The other implication of the bug (I presume) is that you cannot remove custom directories from the slideshow. Does anyone know of a workaround to accomplish this?
Comment 16 Mircea Kitsune 2014-02-16 11:59:12 UTC
(In reply to comment #15)

I believe editing the settings file manually (I forgot which) is the only way unfortunately.
Comment 17 sparhawk 2014-02-17 00:51:43 UTC
Ah ok, thanks. I grepped through `~/.kde/share` and found a reference to the path in `~/.kde/share/config/plasma-desktop-appletsrc`

I was incorrect previously; this file only contains the last entry that I specified in the settings. The reason why I thought the previous entries persisted is that there seems to be another bug where the specified directory is not the only one that is included in the slideshow.

I'm trying to workaround bug 325380, where the image is not reloaded when changed. I've set up a slideshow pointing to a directory containing a single symlinked file, but it appears to traverse into the directory and subdirectories of the target of the symlink. I've created bug 331223.
Comment 18 Shriramana Sharma 2014-02-17 02:09:02 UTC
Here's a side-effect (I think) of the bug (or please tell me if it should be reported as a separate bug). When we open the Desktop Settings and *don't* see the custom folders listed, if we change the positioning/colour/time settings in the same pane and hit OK/Apply, then apparently the non-displayed folders are forgotten and it goes back to the system default wallpapers or something. Anyone else can confirm it? And developers please tell me whether I should report it as separate bug or not? (It seems to be a side-effect of this one.)
Comment 19 sparhawk 2014-02-17 02:27:38 UTC
Shriramana, yes, I think I see the same thing. I'm pretty sure it's just a side effect of this bug.
Comment 20 TOM Harrison 2014-02-23 07:42:01 UTC
i have just found when init.
the m_dirs has value
when the Image::createConfigurationInterface.
m_dirs did not has value.
Comment 21 TOM Harrison 2014-02-23 08:25:04 UTC
i have temporary solved this problem just move the m_dirs to the global variable, and it solved.
waiting for your solution.
Comment 22 TOM Harrison 2014-02-23 10:58:47 UTC
i know why it not list.
m_dirs did not list because
BackgroundDialog::changeBackgroundMode 483
d->wallpaper->setPreviewing(true);
this
but the slideshow did not preview.
this code will cause slideshow not init the m_dirs, causing the slide did not show configure
Comment 23 TOM Harrison 2014-02-23 11:09:12 UTC
Created attachment 85287 [details]
diff that fix this BUG.

the "if (mode == "SingleImage")"
i didn't know that is correct or not.
Comment 24 François Valenduc 2014-02-23 11:28:21 UTC
I tried the patch and unfortunately, it didn't change anything to the problem.
Comment 25 TOM Harrison 2014-02-23 11:53:23 UTC
(In reply to comment #24)
> I tried the patch and unfortunately, it didn't change anything to the
> problem.

you can change the code by yourself, the modify is simple.
and this patch is form kde 4.11.6

i change this code
libs/plasmagenericshell/backgrounddialog.cpp:480


change 
         d->wallpaper->setPreviewing(true);
         d->preview->setPreview(d->wallpaper);
to

      if (mode == "SingleImage") {
         d->wallpaper->setPreviewing(true);
         d->preview->setPreview(d->wallpaper);
      }
Comment 26 TOM Harrison 2014-02-23 11:54:19 UTC
and this patch is form kde 4.11.6 -> and this patch is form kde-workspace 4.11.6 
the environment is kde 4.12.2
Comment 27 François Valenduc 2014-02-23 12:11:57 UTC
That's what I did and it did not change anything.
Comment 28 TOM Harrison 2014-02-23 12:16:07 UTC
(In reply to comment #27)
> That's what I did and it did not change anything.

did you have restart the plasma-desktop after compile.
and you need something settings in slideshow to test.
i am making a video. i will poset the url later
Comment 29 TOM Harrison 2014-02-23 12:17:30 UTC
another thing, the cmake of kde-workspace did not install the library correct.
i link it to /usr/lib manually to let the patch work.
Comment 30 François Valenduc 2014-02-23 12:30:48 UTC
I am using gentoo.  I made a patch on basis of the kde-workspace git tree and I added it to a modified ebuild. Then I recompiled plasma-workspace and I restarted kdm. And the problem still occurs.
Comment 31 TOM Harrison 2014-02-23 12:37:34 UTC
http://www.mediafire.com/watch/3i3xw37wlwmbupd/out(2).ogv
this is video tutorial about solved problem
Comment 32 TOM Harrison 2014-02-23 12:40:08 UTC
(In reply to comment #30)
> I am using gentoo.  I made a patch on basis of the kde-workspace git tree
> and I added it to a modified ebuild. Then I recompiled plasma-workspace and
> I restarted kdm. And the problem still occurs.

than you maybe need to check that the code is change or not?
my patch is just create for see the diff :)
you need to into kde-workpace
Comment 33 François Valenduc 2014-02-23 13:09:11 UTC
The code is well modified like this in /var/tmp/portage/kde-base/plasma-workspace-4.11.5/work/plasma-workspace-4.11.5/libs/plasmagenericshell/backgrounddialog.cpp :

  if (mode == "SingleImage") {
        d->wallpaper->setPreviewing(true);
        d->preview->setPreview(d->wallpaper);
      }

And the problem still occurs.
Comment 34 TOM Harrison 2014-02-23 13:16:48 UTC
(In reply to comment #33)
> The code is well modified like this in
> /var/tmp/portage/kde-base/plasma-workspace-4.11.5/work/plasma-workspace-4.11.
> 5/libs/plasmagenericshell/backgrounddialog.cpp :
> 
>   if (mode == "SingleImage") {
>         d->wallpaper->setPreviewing(true);
>         d->preview->setPreview(d->wallpaper);
>       }
> 
> And the problem still occurs.

that weird, i am sure this problem is solved in ubuntu 13.10 kde 4.12.2
i also install a new kubuntu in the virtual machine to debug this problem
maybei need to down a arch linux to test.

one think you must sure that is the library must be correct.
i have a problem about after compiling the kde-workspace, the install location is not correct.
Comment 35 TOM Harrison 2014-02-23 13:30:00 UTC
did you have arch linux iso url.
the official release just has pure command, and i did not know how to install.
Comment 36 François Valenduc 2014-02-23 13:38:19 UTC
I said  in comment 32 that I use gentoo. I also have ubuntu. So, I am rebuilding the package now with the patch to see what happens.
Comment 37 TOM Harrison 2014-02-23 13:41:06 UTC
sorry, my fault.
i have do a trick that is i just compile the libs/plasmagenericshell/ this code and replace, so i am actually did not know compile the all kde-workspace will compile that library or not.

maybe you can try that just compile that library
Comment 38 François Valenduc 2014-02-23 13:47:55 UTC
And what is the name of this library so that I can find which package needs to be recompiled ?
Comment 39 François Valenduc 2014-02-23 14:19:27 UTC
I found out that I needed to recompile the package "libplasmagenericshell". And the patch did indeed solved the problem.

Thanks for your help.
Comment 40 Mircea Kitsune 2014-02-23 19:07:15 UTC
Thanks for finding the solution. Hopefully one of the admins notices this and can add the code change before next release.
Comment 41 markuss 2014-02-23 19:28:55 UTC
I'm pretty sure nobody will notice without a Review Request at https://git.reviewboard.kde.org/
Comment 42 TOM Harrison 2014-02-23 23:54:14 UTC
this problem is seems that the background dialog force the dir list is preview.
but the slideshow did not need preview, so that code will cause that kdirwatch did not update the m_dirs.
Comment 43 Alexander Blinne 2014-02-27 20:09:52 UTC
Created attachment 85346 [details]
According to the comments, this solves the problem

I am re-adding the attached patch, because it has not been recognized as such by the bugtracker. Maybe in this form someone will notice it and apply it.
Comment 44 Christoph Feck 2014-03-10 00:18:27 UTC
*** Bug 331923 has been marked as a duplicate of this bug. ***
Comment 45 Mark C 2014-03-10 12:02:06 UTC
Bug still persists in 4.2.13
Comment 46 Christoph Feck 2014-03-26 22:57:30 UTC
In order to apply the patch to the KDE repositories, we need the name of its author. Does comment #42 mean there is a regression with the patch?
Comment 47 TOM Harrison 2014-03-27 01:22:17 UTC
(In reply to comment #46)
> In order to apply the patch to the KDE repositories, we need the name of its
> author. Does comment #42 mean there is a regression with the patch?

according to my check. it seems the problem with preview.
i just add an if to solve this problem. 

the patch is upload by me, my English name "TOM Harrison"
Comment 48 zhcj 2014-04-06 22:08:10 UTC
I am using gentoo,bug still persists in 4.2.14.
According to #Comment 39 ,I recompile the package "libplasmagenericshell" and  solved it.
thanks!
Comment 49 Christoph Feck 2014-04-20 12:56:40 UTC
I am a bit hesitant to apply the patch, because I feel the regressions were caused by changes to files in plasma/generic/wallpapers/image instead of libs/plasmagenericshell.

Could you please create a review request at https://git.reviewboard.kde.org/ and explain your code changes there for others to review?
Comment 50 François Valenduc 2014-04-20 13:05:33 UTC
As I said in comment #39, this patch solves the problem and I didn't notice any regression since I applied it already almost 2 month ago. Maybe the explanations of the author of this patch are not completely clear, but this patch looks correct.
Comment 51 TOM Harrison 2014-04-20 13:23:29 UTC
according to my gdb trace, the directory list is not work is because the kwatchdir in slideshow will return without init because of preview, so i disable the preview of slideshow.

the original purpose of patch is show this could solve this problem, but i did not know that is the perfect solution.
Comment 52 Mark C 2014-04-20 17:26:40 UTC
Checked in 4.13.0 (archlinux). Still a problem.
Comment 53 François Valenduc 2014-05-15 15:09:16 UTC
This bug is solved by a patch available since more than 2 months (see comment #30). How long will it still take before that this patch is finally included in KDE ?
Comment 54 Christoph Feck 2014-05-15 18:43:58 UTC
François, without going through review, Plasma developers will not see the patch. I am not going to approve it as stated in comment #49.
Comment 55 Mircea Kitsune 2014-05-15 18:51:37 UTC
Although I don't want to take this off topic, it's disappointing to see an important project like KDE having so much trouble getting even the tiniest issues solved... even when a patch has been posted for months. I hope someday testing and patch reviews can receive a reform. Release versions of KDE are full of little bugs, this being one of them... and although I can live with most myself, others might stop considering it a stable software package any longer.
Comment 56 Christoph Feck 2014-05-15 19:12:18 UTC
To get a patch applied, we accept multiple workflows, and I will explain all of them to you.

1. Attach a patch to the bug report. Has been done here, but requires someone seeing the patch, evaluating it, and take responsibility applying it. I do not know the code enough to decide if it is correct, so I asked for a review.

2. Create a review request, so that code can be audited by developers before it is applied. Usually those who review the patch are also willing to apply it, if the submitter has no commit rights.

3. Request commit rights, and commit the fix yourself. This requires that you take responsibility for the consequences, if you skipped the review process.

If you feel there should be an additional supported option, or the existing options need a "reform", please discuss changes to code review & commit policies in the KDE forums or on the kde-core-devel mailing list. The usual answer (if any) will be "if you feel you can help us doing better, please step up doing so!"
Comment 57 François Valenduc 2014-05-15 19:26:03 UTC
I don't know the way the KDE team handle patches and I am not questionning it. However, the author of the patch doesn't seem in a hurry to submit a review request. So, the problem persists since a long time altough the patch solves the problem apparently without introducing regressions. I don't see any adverse effect with this patch. So can anybody else than the patch author ask for a review request ?
Comment 58 Mircea Kitsune 2014-05-15 19:28:53 UTC
(In reply to comment #56)
Fair enough. I didn't mean to blame anyone of course. Just a little confused that little bugs like this can take so long to solve in such an important project... and especially that they make it in release versions at all. That clears it up a bit more... and yes, I wish I knew more programming to help more too.
Comment 59 TOM Harrison 2014-05-16 11:46:19 UTC
I do not know how to post a review, so I just continue re-use this patch to let the slideshow dir work.
Comment 60 TOM Harrison 2014-05-16 12:50:45 UTC
It show this
The selected file does not appear to be a diff.
I cannot submit a review :(
I did not know how to use that :)
Comment 61 François Valenduc 2014-05-16 17:16:24 UTC
Created attachment 86667 [details]
patch maybe in a better format

This is a patch obtained with git diff. Maybe it will be better detected as a patch.
Comment 62 TOM Harrison 2014-05-17 01:30:17 UTC
now is this = =.
The file 'libs/plasmagenericshell/backgrounddialog.cpp' (r645de3f) could not be found in the repository
Comment 63 TOM Harrison 2014-05-17 01:45:15 UTC
OK, I success, I forget to choose the right repository.
I take time to choose the repository
Comment 64 Christoph Feck 2014-05-17 18:02:40 UTC
*** Bug 334926 has been marked as a duplicate of this bug. ***
Comment 65 Christoph Feck 2014-05-21 20:34:56 UTC
*** Bug 335115 has been marked as a duplicate of this bug. ***
Comment 66 Christoph Feck 2014-07-20 17:21:56 UTC
*** Bug 337619 has been marked as a duplicate of this bug. ***
Comment 67 François Valenduc 2014-08-20 18:56:18 UTC
Sorry to be a bit impolite but the fact that this problem is not solved also a patch is available for month is becoming completely ridiculous in my opinion.
I have already said several times that the patch provided here solves the problem without any regression.

Why wait so long to apply it ? The authors has done testing with gdb and analysed the code to come up with a solution. I don't understand what you still want to be convinced. Maybe the explanation are not in perfect english but is it a blocking factor ?
Comment 68 Christoph Feck 2014-08-20 19:13:44 UTC
Git commit d4fdd1a78821884cf515467cdbbb769096033452 by Christoph Feck, on behalf of Tom Harrison.
Committed on 20/08/2014 at 19:10.
Pushed by cfeck into branch 'KDE/4.11'.

Fix custom folder list in slideshow wallpaper config

REVIEW: 118180
FIXED-IN: 4.11.12

M  +4    -2    libs/plasmagenericshell/backgrounddialog.cpp

http://commits.kde.org/kde-workspace/d4fdd1a78821884cf515467cdbbb769096033452
Comment 69 Christoph Feck 2014-08-20 19:20:45 UTC
Just committed it for the September KDE Workspace release. Sorry if someone got the impression that I wanted to block it.

Thanks to Tom for the investigation, the patch, and the PATIENCE :)
Comment 70 john 2015-01-02 20:47:34 UTC
Bug persists in Kubuntu 14.04 LTS.
Comment 71 Christoph Feck 2015-01-03 00:09:20 UTC
Please check if you actually have Plasma 4.11.12 using "plasma-desktop --version".