<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://reviewboard.kde.org/r/5172/">http://reviewboard.kde.org/r/5172/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">isAlwaysReturnPreferredSearchProvidersOn is quite a mouthful. And yet, I would use "Enabled" instead of "On", to be more Qt-like.
Thinking about all this, there are really two use cases for KUriFilter: "the user's text -might- be a candidate for uri filtering" (e.g. konqueror or krunner), and "the user wants this to be uri-filtered, including web-search fallback" (e.g. konversation's popupmenu; pasting onto khtml; any use case not based on a location bar, really, where typos can't happen (*)).
Your setter is really about choosing between these two modes. Maybe it could have a better name, like setForceFiltering()? or more precise: setWebSearchFallbackEnabled()?
(*) iirc one of the reasons why the fallback-to-web-search was disabled in konqueror's location bar is that typing /ect instead of /etc would trigger a google search for /ect, unexpectedly.
Even the need for setWebSearchFallbackEnabled() would have been unexpected to Eike, but well, we have to cover both use cases and the only way to do that is to have a setter; and for compatibility it has to be off by default.
=> Ship It, but with a better method name if possible.</pre>
<br />
<p>- David</p>
<br />
<p>On August 27th, 2010, 11:09 p.m., Dawit Alemayehu wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://reviewboard.kde.orgrb/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.</div>
<div>By Dawit Alemayehu.</div>
<p style="color: grey;"><i>Updated 2010-08-27 23:09:20</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;">The attached patch does the following:
#1. Adds setAlwaysReturnPreferredSearchProvidersOn/isAlwaysReturnPreferredSearchProvidersOn to ensure that the kuriikwsfilter plugin returns the preferred provider search engine list when no default search engine is specified. Currently it fails to do so and that behavior will not change unless you set the flag using the aforementioned function.
With this change instead of having to do the following:
KUriFilterData filterData(foo);
filterData.setAlternateDefaultSearchProvider("google"); // hmm... would google be there ?
if (KUriFilter::self()->filterUri(filterData, QStringList() << "kuriikwsfilter")
// do something here...
With
KUriFilterData filterData(foo);
filterData.setAlwaysReturnPreferredSearchProvidersOn(true);
if (KUriFilter::self()->filterUri(filterData, QStringList() << "kuriikwsfilter")
// do something here...
#2. Deprecates the newly added function filterSearchUri(KUriFilterData&) in favor of one that accepts an enum filterSearchUri(KUriFilterData&, SearchFilterTypes types). That way instead of the caller having to know what "kuriikwsfilter' or 'kurisearchfilter' plugins do, they can specify "NormalTextFilter" or "WebShortcutFilter" or an OR combination of both. This change makes it possible to do search filtering without having to worry about which filters to use. For example, the last if statement from the above example would become:
if (KUriFilter::self()->filterSearchUri(filterData, KUriFilter::NormalTextFilter))
// do something here...</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>/trunk/KDE/kdebase/runtime/kurifilter-plugins/ikws/kuriikwsfilter.h <span style="color: grey">(1168549)</span></li>
<li>/trunk/KDE/kdebase/runtime/kurifilter-plugins/ikws/kuriikwsfilter.cpp <span style="color: grey">(1168549)</span></li>
<li>/trunk/KDE/kdelibs/kio/kio/kurifilter.h <span style="color: grey">(1168902)</span></li>
<li>/trunk/KDE/kdelibs/kio/kio/kurifilter.cpp <span style="color: grey">(1168902)</span></li>
</ul>
<p><a href="http://reviewboard.kde.org/r/5172/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>