<table><tr><td style="">dfaure requested changes to this revision.<br />dfaure added a comment.<br />This revision now requires changes to proceed.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D27148">View Revision</a></tr></table><br /><div><div><p>statJob->exec() creates a nested event loop, which processes timers and socket events etc. -- this can create unexpected re-entrancy and is a nasty source of bugs.<br />
Therefore it should be avoided as much as possible, *especially* in libraries which don't have the full picture about what happens in the application.</p>

<p>The only safe way to do this is with an async job, i.e. signals and slots. This is tricky in general (when the caller expects things to happen immediately), but by luck this isn't the case here. Apps expect KParts to download remote files. So what could be done, is a statjob before the download, and if stat says there's a local path KParts can *then* set m_file instead of downloading.</p>

<p>But wait.... KParts::ReadOnlyPart::openUrl already does *exactly* that.<br />
See the code under</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">// Maybe we can use a "local path", to avoid a temp copy?</pre></div>

<p>which uses KIO::mostLocalUrl() the right way, async.</p>

<p>So I have to ask... what is this fixing, exactly? Why isn't openUrl called, or why doesn't it go into that code path, in your use case?</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R306 KParts</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D27148">https://phabricator.kde.org/D27148</a></div></div><br /><div><strong>To: </strong>pdabrowski, elvisangelaccio, ngraham, Frameworks, dfaure<br /><strong>Cc: </strong>marten, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns<br /></div>