Bug 274501

Summary: Konsole's profiles apply buttons are always active
Product: [Applications] konsole Reporter: blibberblubb <twinbase>
Component: generalAssignee: Konsole Developer <konsole-devel>
Status: RESOLVED FIXED    
Severity: normal CC: adaptee, lueck, rsimmons0
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Gentoo Packages   
OS: Linux   
Latest Commit: Version Fixed In: 4.8
Attachments: apply button should be active only when these is really modification
apply button should be active only when these is really modification
apply button should be active only when these is really modification
apply button should be active only when these is really modification

Description blibberblubb 2011-05-30 09:30:53 UTC
Version:           unspecified (using Devel) 
OS:                Linux

The apply buttons of
"Settings -> Configure Current Profile" and "Configure Profiles"
stay always active, even on unchanged settings and no matter how often you "apply".


Reproducible: Always
Comment 1 Burkhard Lück 2011-05-31 08:09:37 UTC
Confirmed using master compiled from sources and kubuntu 10.10 with kde 4.6.2
Comment 2 Robert Simmons 2011-06-17 18:36:23 UTC
I can confirm that this bug exists in Kubuntu 11.04 with KDE 4.6.4

I have found thing bug in many different apps,  I think it is specific to Kubuntu since people running OpenSuse are not affected by it.

I am going to open a Kubuntu bug and link all the bugs together.
Comment 3 Burkhard Lück 2011-06-18 12:40:35 UTC
Checking with a openSUSE 11.4 kde 4.6.0 in a virtual machine, the "Apply" button is always enabled, and never is greyed out even if I press it.

So I doubt it is a Kubuntu issue.
Comment 4 Robert Simmons 2011-06-19 06:14:01 UTC
I guess it isn't specific to Kubuntu, oops.
Comment 5 Jekyll Wu 2011-07-06 17:27:36 UTC
Created attachment 61650 [details]
apply button should be active only when these is really modification

I can confirm this.  

I have created a patch to ensure that the `apply` button is active only when the user has really made some change . The patch is against master branch.
Comment 6 Jekyll Wu 2011-07-07 04:54:04 UTC
Created attachment 61664 [details]
apply button should be active only when these is really modification

Previous patch does not reset apply button after it has been clicked.
This one also fix that.
Comment 7 Kurt Hindenburg 2011-07-07 14:33:39 UTC
#6 thanks for the patch.  Did you happen to look at using KDialog::enableButtonApply	(	bool 	state	 )?

Also, there's something going on trying to change the Appearance->Scheme.  At first it won't allow any changes (and the Apply is disabled).  Perhaps, because it tries to do a preview?  After messing around I can eventually get it to change.
Comment 8 Jekyll Wu 2011-07-08 11:42:03 UTC
Created attachment 61703 [details]
apply button should be active only when these is really modification

(In reply to comment #7)
> 
>
Thanks for that tip. I really should have read more documents before coding.

This patch now works with appearance previews. However, there is a subtle
problem with antialias property. It does not work well at first place.

When the antialias property is changed in the sequence of 'on->off->on', the
`apply` button will be active, which is unexpected. But after being clicked once
to save that non-existing change , the `apply` button will work as expected, being active only when change really exist.

I can't figure out why it behave in that weird way.
Comment 9 Kurt Hindenburg 2011-07-08 15:17:28 UTC
#8, I would guess because you are comparing QFonts.  I'm find w/ committing it w/ this issue if that is the only one found.


Also, you can remove the include for KPushButton and try to watch the whitespace-only changes.

Thanks.
Comment 10 Jekyll Wu 2011-07-08 16:27:58 UTC
Created attachment 61710 [details]
apply button should be active only when these is really modification

(In reply to comment #9)
> #8, I would guess because you are comparing QFonts.  I'm find w/ committing it w/ this issue if that is the only one found.   Also, you can remove the include for KPushButton and try to watch the whitespace-only changes.  Thanks.

Extra #include and whitespace changnes are removed. Also, the internal method is renamed for consistency.

The code does compare QFont, but it is only because the object within QVariant happens to be QFont. If I did not misunderstand, does your reply imply unless extra specific code is added to deal with the comparison between QFonts, that
weird issue can not be solved?
Comment 11 Kurt Hindenburg 2011-07-08 19:23:24 UTC
You get the same behavior if you change the font via 'Edit Font' and then change it back to the original.
It is a minor issue.

Do you have the ability to commit to git master?
Comment 12 Jekyll Wu 2011-07-09 06:16:41 UTC
(In reply to comment #11)
> Do you have the ability to commit to git master?

No. If you think this patch is acceptable, feel free to make any needed modification and commit it. Thanks.
Comment 13 Kurt Hindenburg 2011-07-16 15:46:53 UTC
Git commit ecfb037c27b84e38b2a1a4d6c3974a4505434be7 by Kurt Hindenburg.
Committed on 16/07/2011 at 17:44.
Pushed by hindenburg into branch 'master'.

Enable Edit Profile Apply button only when profile has changed.

Keep track of any changes to the Profile and only enable the Apply
button when something has really changed.

Patch by Jekyll Wu adaptee@gmail.com

Might backport to 4.7 if there are no concerns.
BUG: 274501
FIXED-IN: 4.8
REVIEW: 101904

M  +90   -44   src/EditProfileDialog.cpp
M  +9    -0    src/EditProfileDialog.h

http://commits.kde.org/konsole/ecfb037c27b84e38b2a1a4d6c3974a4505434be7