Bug 335709 - [frameworks] Saving settings does not work
Summary: [frameworks] Saving settings does not work
Status: RESOLVED FIXED
Alias: None
Product: dolphin
Classification: Applications
Component: general (show other bugs)
Version: 4.60
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: Dolphin Bug Assignee
URL:
Keywords:
: 335812 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-06-02 17:56 UTC by Martin Klapetek
Modified: 2014-06-05 22:57 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Quick Hack (2.69 KB, patch)
2014-06-03 19:58 UTC, Frank Reininghaus
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Klapetek 2014-06-02 17:56:57 UTC
Changing and saving any Dolphin setting in frameworks version has no effect at all; no related output in terminal.
Comment 1 Frank Reininghaus 2014-06-02 21:21:58 UTC
Thanks for the bug report. I can confirm the problem.

I see that Alexander commented out some lines in DolphinSettingsDialog as part of the Qt5/KF5 port and added a "TODO: port" note. I'm not sure if that is related to the problem - I never really worked intensively with the settings code myself, and the review request

https://git.reviewboard.kde.org/r/117395/

does not mention the issue. I must have missed it when looking at the patch - it was just too big ;-)

Unfortunately, a quick search in

http://community.kde.org/Frameworks/Porting_Notes

did not show me anything helpful. Maybe I or someone else will have to have a closer look at some point. Any help would be appreciated.
Comment 2 Frank Reininghaus 2014-06-02 21:56:37 UTC
Hm, DolphinSettingsDialog::slotButtonClicked(int button) is not called when clicking OK/Apply in frameworks, The reason seems to be that KPageDialog does not inherit KDialog (which emits the buttonClicked signal) any more in KF5. Ouch.

I can imagine that we are not the only application which will have this problem. Should be easy to fix though.
Comment 3 Frank Reininghaus 2014-06-03 19:58:08 UTC
Created attachment 87000 [details]
Quick Hack
Comment 4 Frank Reininghaus 2014-06-05 06:24:54 UTC
*** Bug 335812 has been marked as a duplicate of this bug. ***
Comment 5 Martin Klapetek 2014-06-05 11:00:48 UTC
I can confirm the patch in comment #3 works.
Comment 6 Frank Reininghaus 2014-06-05 21:33:44 UTC
Thanks for testing it! I modified the patch slightly: I removed the bool argument of the slots - somehow I had thought that this was needed when connecting to the clicked(bool) signal of the button with the new connect syntax, but it seems to work even if I drop the argument.

This makes the patch even smaller, and still works fine for me :-)

https://git.reviewboard.kde.org/r/118576/

I'll push it soon if no problems are found.
Comment 7 Frank Reininghaus 2014-06-05 22:57:52 UTC
Git commit e03ed3ac14a543b62ed61a3bfabb106342d3f6ac by Frank Reininghaus.
Committed on 05/06/2014 at 22:51.
Pushed by freininghaus into branch 'frameworks'.

Make the settings dialog work in the frameworks branch

The KF5 version of KPageDialog has no virtual slot
"slotButtonClicked(int button)". Its kdelibs 4.x counterpart had such
a slot, which was connected automatically to the corresponding signal.

This slot was overriden by

DolphinSettingsDialog::slotButtonClicked(int button)

which was responsible for applying the changed setting and restoring
the default values if the corresponding button was clicked.

The lack of the buttonClicked(int) signal and the corresponding slot
caused the problem that clicking a button in the settings dialog had no
effect.

This patch makes the functions applySettings() and restoreDefaults()
functions slots, and connects them directly to the "clicked" signal of
the corresponding buttons.
REVIEW: 118576

M  +4    -12   dolphin/src/settings/dolphinsettingsdialog.cpp
M  +0    -6    dolphin/src/settings/dolphinsettingsdialog.h

http://commits.kde.org/kde-baseapps/e03ed3ac14a543b62ed61a3bfabb106342d3f6ac