<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/100154/">http://git.reviewboard.kde.org/r/100154/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 19th, 2010, 10:58 p.m., <b>Jon Ander Peñalba</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think that this solves the problem. I don't know if I've added all the symbols that cause trouble, but if not, it's really simple to update :)</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">In my opinion, it is just the wrong way to solve the problem. You still have not addressed the case where you match "XX" in a string of "XXXXXXXXXXXXX".
In you last patch, you replace special symbols, but those can appear in an URL, you want to match them. If you want to escape all the control character in a meaningful way, your code will end up being bigger than the previous code.
The objective of you patch is "Text highlighting in the url bar simplified", the new code is neither simplified, neither correct.</pre>
<br />
<p>- Benjamin</p>
<br />
<p>On November 19th, 2010, 10:56 p.m., Jon Ander Peñalba wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.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 rekonq.</div>
<div>By Jon Ander Peñalba.</div>
<p style="color: grey;"><i>Updated 2010-11-19 22:56:28</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;">I've simplified text highlighting. I think the behaviour hasn't changed, but a second opinion is welcome :)
I've removed the Qt::escape (and updated the test accordingly) because I find it useless, but if it needs to be there for any reason there's no problem in putting it back.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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 'listitem_test' test passes.</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>src/tests/listitem_test.cpp <span style="color: grey">(fc0b62e)</span></li>
<li>src/urlbar/listitem.cpp <span style="color: grey">(a0462e7)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100154/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>