Bug 268898

Summary:	KLineEdit: Clear button is invisible if the text is set before setClearButton(true) is called (regression since 4.5)
Product:	[Unmaintained] kdelibs	Reporter:	Frank Reininghaus <frank78ac>
Component:	kdeui	Assignee:	kdelibs bugs <kdelibs-bugs>
Status:	RESOLVED FIXED
Severity:	normal	CC:	anthony.renoux, cfeck, fischer, frank78ac, hugo.pereira.da.costa, hugo
Priority:	NOR	Keywords:	investigated
Version:	SVN
Target Milestone:	---
Platform:	Compiled Sources
OS:	Linux
Latest Commit:		Version Fixed In:
Sentry Crash Report:
Attachments:	Patch for klineedittest that makes the bug visible possible patch updated patch

Description Frank Reininghaus 2011-03-19 14:11:39 UTC

Created attachment 58166 [details]
Patch for klineedittest that makes the bug visible

Version:           SVN (using Devel) 
OS:                Linux

If you create a KLineEdit and call setClearButton(true) *after* the text has been set, the clear button is not shown. It can be clicked though.
If the text is cleared and then some new text is entered, the button is shown.


Reproducible: Always

Steps to Reproduce:
Two ways to reproduce:

1. Apply the attached patch to klineedittest. The patch makes sure that the line edit's text is set before setClearButton(true) is called.

2. In the 4.5 branch, open Dolphin, press F10 to create a folder -> the bug can be noticed in the dialog asking for the folder name. This cannot be reproduced in 4.6 or master due to other changes.



Actual Results:  
Clear button is not visible (but it can be clicked).

Expected Results:  
Clear button should be visible.

This is a regression since KDE 4.5. I've found out with git bisect that the bad commit is

https://projects.kde.org/projects/kde/kdelibs/repository/revisions/ab3b5ed72c27ed83bffdfa753a84938df63dde4c

Comment 1 Frank Reininghaus 2011-03-19 14:15:00 UTC

CC'ing Hugo (who committed the patch that looks like it's caused the regression).

Comment 2 Frank Reininghaus 2011-03-19 14:16:35 UTC

I've just seen in the logs that this bug had been there even earlier and was fixed already: bug 193045. It would be nice if we could unit-test it ;-)

Comment 3 Christoph Feck 2011-07-24 00:54:01 UTC

Hugo, any comment?

Comment 4 Hugo Pereira Da Costa 2011-07-24 09:15:25 UTC

Will have a look ASAP.
Thanks for cc-ing me.

Comment 5 Hugo Pereira Da Costa 2011-07-25 14:56:17 UTC

Created attachment 62182 [details]
possible patch

ok. So attached is a patch, that apparently works (feel free to double check and report back).

Now, digging into the code, it seems to me that the logic could be simplified/clarified (notably there are a number of redundant calls to button->show() and button->hide() at initialization), and I might come with more substancial changes to review board.

Will have some fix pushed for kde 4.7.1
(I dont think this is a serious enough issue to push a fix in 4.7.0)

Comment 6 Hugo Pereira Da Costa 2011-07-26 21:44:28 UTC

Created attachment 62222 [details]
updated patch

updated version of the patch.
I like this one better than the first because 

- it actually fixes the logic (which was flawed IMHO) in KLineEditButton::animateVisible

- it consequently simplifies the logic in KLineEdit::updateClearButtonIcon

And the "klineedittest" unit-test works with and without the patch posted in comment #1.

Comments are welcome, but I'll file a ReviewBoard request anyway.

Comment 7 Frank Reininghaus 2011-07-27 15:40:33 UTC

Hugo, thanks for the patch. It fixes the issue for me, and I haven't seen any regressions so far.

@Christoph: Thanks for CC-ing Hugo properly, it seems that I got the e-mail address wrong.

Comment 8 Hugo Pereira Da Costa 2011-07-28 12:54:19 UTC

ok. Pushed.
(see http://git.reviewboard.kde.org/r/102095/)

So closing.
(I'll backport to 4.7.1 too)

Comment 9 Thomas Fischer 2012-12-31 19:18:24 UTC

*** Bug 294954 has been marked as a duplicate of this bug. ***