<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/114253/">http://git.reviewboard.kde.org/r/114253/</a>
</td>
</tr>
</table>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/114253/diff/3/?file=222675#file222675line39" style="color: black; font-weight: bold; text-decoration: underline;">plasma/generic/dataengines/favicons/favicons.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">35</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">FaviconProvider</span> <span class="o">*</span><span class="n">provider</span> <span class="o">=</span> <span class="k">new</span> <span class="n">FaviconProvider</span><span class="p">(</span><span class="k">this</span><span class="p">,</span> <span class="n">identifier</span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">39</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">FaviconProvider</span> <span class="o">*</span><span class="n">provider</span> <span class="o">=</span> <span class="k">new</span> <span class="n">FaviconProvider</span><span class="p">(</span><span class="k">this</span><span class="p">,</span> <span class="n"><span class="hl">QUrl</span></span><span class="p"><span class="hl">(</span></span><span class="n">identifier</span><span class="p">)<span class="hl">)</span>;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hmm, this has simply moved the problem here.
But now I see: this "identifier" actually comes from FaviconProvider in the first place, so it has full control over that string?
In that case, having the conversion to QUrl should indeed be closer to the conversion from QUrl to QString, i.e. inside Provider, as it was originally.
But I'm missing something, this is a chicken an egg situation. Who creates this identifier initially, before the Provider is created in the first place?
In other words, who calls sourceRequestEvent?
Sorry I don't know anything about data engines - but I'm smelling a possible QUrl misuse somewhere here.
</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/114253/diff/3/?file=222675#file222675line44" style="color: black; font-weight: bold; text-decoration: underline;">plasma/generic/dataengines/favicons/favicons.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">40</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">provider</span><span class="o">-></span><span class="n">image</span><span class="p">()</span> <span class="o">!=</span> <span class="n">QImage</span><span class="p">())</span> <span class="p">{</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">44</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">provider</span><span class="o">-></span><span class="n">image</span><span class="p">()</span> <span class="o">!=</span> <span class="n">QImage</span><span class="p">())</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">unrelated, but this should most certainly be .isNull() instead of creating a temporary QImage just for comparison purposes.</pre>
</div>
<br />
<p>- David Faure</p>
<br />
<p>On December 5th, 2013, 10:48 a.m. UTC, Andrea Scarpino wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks and Plasma.</div>
<div>By Andrea Scarpino.</div>
<p style="color: grey;"><i>Updated Dec. 5, 2013, 10:48 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kde-workspace
</div>
<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;">= subject.
I replaced KUrl by QUrl and KStandardDirs by QStandardPaths. Please check if I replaced them correctly.</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;">Builds.</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>plasma/generic/dataengines/CMakeLists.txt <span style="color: grey">(3e94325)</span></li>
<li>plasma/generic/dataengines/favicons/CMakeLists.txt <span style="color: grey">(4af0f14)</span></li>
<li>plasma/generic/dataengines/favicons/faviconprovider.h <span style="color: grey">(1f370f9)</span></li>
<li>plasma/generic/dataengines/favicons/faviconprovider.cpp <span style="color: grey">(bc72a66)</span></li>
<li>plasma/generic/dataengines/favicons/favicons.h <span style="color: grey">(79565bf)</span></li>
<li>plasma/generic/dataengines/favicons/favicons.cpp <span style="color: grey">(2eae673)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/114253/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>