Review Request: Fix keypress stealing issue

[David Faure]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101491/#review3704
-----------------------------------------------------------

Ship it!


Patch is okay, I didn't realize the issue was that simple to fix ;)

I agree that, especially in a component like khtml, all shortcuts should probably be limited to "when the part widget has focus", like you suggest.

About the second question, here's my commit log for 60ae8626e917cb1f3:

    Re-route the old actions "Find Text as You Type" and "Find Links as You Type" to the findbar.
    Remove them from the menu, since they are redundant with "Find text", but kept two actions
    just for the shortcuts:
    
    '/' will still activate "find text (not just links)", but the shortcut for "find links" is removed by default,
    advanced users will have to set one again. Otherwise newbies would get confused after pressing the shortcut
    (was single-quote) by mistake: Esc and Ctrl+F would still find in links only.

=========

No strong opinion though, on whether it's really needed to keep the '/' shortcut for compatibility. Let's email all KDE users and ask them... :-)

- David


On June 2, 2011, 8:38 a.m., Thomas Friedrichsmeier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101491/
> -----------------------------------------------------------
> 
> (Updated June 2, 2011, 8:38 a.m.)
> 
> 
> Review request for kdelibs and David Faure.
> 
> 
> Summary
> -------
> 
> Shortcut context was not set correctly for the new "Find Text as You Type" action, which can result in the khtmlpart "stealing" '/'-keypresses from other widgets in the same main window. This patch would fix that in the most straight-forward fashion. However:
> 
> a) I do wonder, why this is not simply set for *all* applicable actions (including those which do not have a shortcut set by default, but might have a user-configured shortcut!). I.e. something like
> 
> foreach (QAction *action, actionCollection()->actions())
> {
>     action->setShortcutContext(Qt::WidgetWithChildrenShortcut);
> }
> 
> But perhaps I am overlooking something?
> 
> b) This is sort of OT, but I could not help wondering:
> I was pretty confused about the difference between Find... and Find Text as You Type. Since Find... really is find-as-you-type, too. Before I finally gathered it from the sources. May I suggest condensing the three actions
> - Find...
> - Find Text as You Type
> - Find Links as You Type
> to two actions
> - Find Text (with default shortcuts Ctrl+F *and* '/')
> - Find Links
> 
> Regards
> Thomas
> 
> 
> Diffs
> -----
> 
>   khtml/khtml_part.cpp ec89b0c8083989afb52ebde714e1fe757ab2e387 
> 
> Diff: http://git.reviewboard.kde.org/r/101491/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Thomas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110606/9a4d518b/attachment.htm>