Bug 268898 - KLineEdit: Clear button is invisible if the text is set before setClearButton(true) is called (regression since 4.5)
Summary: KLineEdit: Clear button is invisible if the text is set before setClearButton...
Status: RESOLVED FIXED
Alias: None
Product: kdelibs
Classification: Frameworks and Libraries
Component: kdeui (show other bugs)
Version: SVN
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: kdelibs bugs
URL:
Keywords: investigated, reproducible
: 294954 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-03-19 14:11 UTC by Frank Reininghaus
Modified: 2012-12-31 19:18 UTC (History)
6 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Patch for klineedittest that makes the bug visible (1.04 KB, text/plain)
2011-03-19 14:11 UTC, Frank Reininghaus
Details
possible patch (1.79 KB, application/octet-stream)
2011-07-25 14:56 UTC, Hugo Pereira Da Costa
Details
updated patch (2.20 KB, application/octet-stream)
2011-07-26 21:44 UTC, Hugo Pereira Da Costa
Details

Note You need to log in before you can comment on or make changes to this bug.
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. ***