Review Request 124089: Connected timeout before timer start.

[David Faure]

> On June 13, 2015, 7:28 p.m., David Faure wrote:
> > startTimer is called many times so doing the connect every time is wrong, and the connect is already done before, so I don't understand this patch.
> > What problem is it supposed to fix?
> 
> Jordan He wrote:
>     I was having an issue with Konsole on Plasma 5 (Kubuntu 15.04). When opening links in Konsole, Konsole would freeze and the link wouldn't open. In Konsole on Plasma 5, type out a link (like "https://www.kde.org"), right click on the link and click "Open Link." Konsole will freeze. This was reproducible for me, and--although I didn't see any other repots on the issue--I decided to investigate it and fix it. Adding this line resolved the issue.
>     The initial call comes from the konsole repository in the slot "UrlFilter::HotSpot::activate". actiate() then calls "new KRun(QUrl(url), activeWindow);"::
>     
>         void UrlFilter::HotSpot::activate(QObject* object)
>         {
>             qDebug() << "Opening URL" << object;
>             QString url = capturedTexts().first();
>     
>             const UrlType kind = urlType();
>     
>             const QString& actionName = object ? object->objectName() : QString();
>     
>             if (actionName == "copy-action") {
>                 QApplication::clipboard()->setText(url);
>                 return;
>             }
>     
>             if (!object || actionName == "open-action") {
>                 if (kind == StandardUrl) {
>                     // if the URL path does not include the protocol ( eg. "www.kde.org" ) then
>                     // prepend http:// ( eg. "www.kde.org" --> "http://www.kde.org" )
>                     if (!url.contains("://")) {
>                         url.prepend("http://");
>                     }
>                 } else if (kind == Email) {
>                     url.prepend("mailto:");
>                 }
>                 QWidget *activeWindow = QApplication::activeWindow();
>                 qDebug() << "Launching KRun with " << url << ", " << activeWindow;
>                 new KRun(QUrl(url), activeWindow);
>             }
>         }
>         
>     Or is the issue really in konsole? Should Konsole create a new instance of ``KRun()`` and then call ``k->init()``?

"new KRun" is the correct way to use KRun with a URL.

(The string concatenation in this code, to make up the URL, will break with special chars and should use KUriFilters, but that's orthogonal).

The KRun constructor calls d->init(), which starts m_timer and connects it to slotTimeout, which the first time, calls KRun::init().

So if this doesn't work correctly in konsole, it must mean that konsole isn't going back to the event loop (so that initial timer can't run). Attach gdb to it to confirm/deny this hypothesis. Otherwise add debug statements within KRun to confirm/deny the various calls I describe above.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124089/#review81450
-----------------------------------------------------------


On June 12, 2015, 8:27 p.m., Jordan He wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124089/
> -----------------------------------------------------------
> 
> (Updated June 12, 2015, 8:27 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> Connected timeout before timer start.
> 
> 
> Diffs
> -----
> 
>   src/widgets/krun.cpp 50660c0a0f103a03d7950aa1f65b60ff796d1825 
> 
> Diff: https://git.reviewboard.kde.org/r/124089/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jordan He
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150615/8c2df191/attachment-0001.html>