Bug 284433 - KTextEdit blocks find shortcut even when find is disabled.
Summary: KTextEdit blocks find shortcut even when find is disabled.
Status: RESOLVED FIXED
Alias: None
Product: kdelibs
Classification: Unmaintained
Component: kdeui (show other bugs)
Version: 4.7
Platform: Unlisted Binaries Linux
: NOR normal
Target Milestone: ---
Assignee: kdelibs bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-19 07:13 UTC by David Edmundson
Modified: 2011-11-14 14:59 UTC (History)
1 user (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Edmundson 2011-10-19 07:13:13 UTC
We use KTextEdit in our application and have
ourTextEdit->enableFindReplace(false);

When pressing control+f with focus on the textedit the shortcut is blocked and does not reach the parent widget, which has a action also using the KStandardShortcut::find(). 

This is because the code in handleShortcut and overrideShortcut reads

if (KStandardShortcut::find().contains(key)) {
       if (findReplaceEnabled)
           parent->slotFind();
       return true;
}

instead of the arguably more correct:

if (KStandardShortcut::find().contains(key)) {
       if (findReplaceEnabled) {
           parent->slotFind();
           return true;
        }
}

which only blocks the event if it's actually handling the shortcut.

Will submit a proper patch on RB if someone else agrees with my reasoning.
Comment 1 Christoph Feck 2011-10-19 09:03:46 UTC
Should be:

    if (findReplaceEnabled && KStandardShortcut::find().contains(key)) {
        parent->slotFind();
        return true;
    }

If you want to go through the trouble to open a review request, please do, otherwise I can just commit it.
Comment 2 David Edmundson 2011-10-19 12:49:29 UTC
Ideally it needs doing for instances of find, find next and replace.

I'm happy for you to commit, or I can do it if you're busy.
Comment 3 Christoph Feck 2011-10-19 14:14:08 UTC
Did you actually test if it works with the change?

I am unsure if KTextEdit::Private::overrideShortcut() needs to be changed, too.
Comment 4 Christoph Feck 2011-10-19 14:15:37 UTC
Doh', you already wrote that it also needs to be changed in overrideShortcut :) I am getting old :P
Comment 5 Christoph Feck 2011-10-19 14:46:11 UTC
https://git.reviewboard.kde.org/r/102919/
Comment 6 Christoph Feck 2011-11-14 14:59:34 UTC
Git commit bb01bd6bcd5f2353c247c382ad716f6afe1a39dc by Christoph Feck.
Committed on 14/11/2011 at 15:56.
Pushed by cfeck into branch 'KDE/4.7'.

Handle shortcuts for Find/Replace actions only when findReplaceEnabled()

Simpler and less intrusive compared to the patch from RB,
as discussed with David Faure.

BUG: 284433
FIXED-IN: 4.7.4
REVIEW: 102919

M  +9    -11   kdeui/widgets/ktextedit.cpp

http://commits.kde.org/kdelibs/bb01bd6bcd5f2353c247c382ad716f6afe1a39dc