<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/101491/">http://git.reviewboard.kde.org/r/101491/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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... :-)</pre>
<br />
<p>- David</p>
<br />
<p>On June 2nd, 2011, 8:38 a.m., Thomas Friedrichsmeier wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs and David Faure.</div>
<div>By Thomas Friedrichsmeier.</div>
<p style="color: grey;"><i>Updated June 2, 2011, 8:38 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>khtml/khtml_part.cpp <span style="color: grey">(ec89b0c8083989afb52ebde714e1fe757ab2e387)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/101491/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>