[KDE/Mac] Review Request 119498: Report crashes of KDE apps in Apple OS X (2) (fix drkonqi)

Tue Jul 29 10:42:24 UTC 2014

> On Juli 27, 2014, 11:17 vorm., Thomas Lübking wrote:
> > drkonqi/main.cpp, line 111
> > <https://git.reviewboard.kde.org/r/119498/diff/1/?file=293511#file293511line111>
> >
> >     This can go unconditionally.
> >     
> >     Show really only shows the window.
> >     Becoming active and then raise for that is WM detail - and WMs have to deal with inadequate raise requests anyway ;-)
> 
> Ian Wadham wrote:
>     I do not understand your comments at all.
>     
>     I just know that the raise() is essential in this context, otherwise the end-user never sees Dr Konqi's window and never investigates or reports the crash.
>     
>     On Apple OS X, an app which is started "properly", by clicking an icon or running an Apple OS X "open" command, will raise the app's window, but Dr Konqi is not being started this way.

You can omit the system test.
It's ok to call "raise()" here on any system.

> On Juli 27, 2014, 11:17 vorm., Thomas Lübking wrote:
> > drkonqi/reportassistantpages_bugzilla.cpp, line 286
> > <https://git.reviewboard.kde.org/r/119498/diff/1/?file=293512#file293512line286>
> >
> >     #ifndef
> 
> Ian Wadham wrote:
>     No, #ifdef. The lines following 286 apply to Apple OS X and nothing else. ATM they are only explanatory. But there is a place to add code if anybody can think of something appropriate (e.g. to check for cookies on the user's browser of choice in a portable way, maybe using Qt).
> 
> RJVB Bertin wrote:
>     Probably not relevant here at all, but mentioning a "place to add appropriate code" for this kind of thing reminds me of the fact that browser plugins are organised in a quite different way on OS X, and it would be great if konqueror could pick them up. Just sayin', for the record :)

I meant to use an #ifndef (and reverse the logic of course) because some #else branch is actually not required - unless of course you indeed want to add some apple specific code.

> On Juli 27, 2014, 11:17 vorm., Thomas Lübking wrote:
> > drkonqi/reportassistantpages_bugzilla.cpp, line 300
> > <https://git.reviewboard.kde.org/r/119498/diff/1/?file=293512#file293512line300>
> >
> >     this is commented.
> >     
> >     if the test is pointless altogether (i do not know. no idea. no record on drkonqui) just remove it with the comment in the commit message, but ifdeffing a void statement makes us look silly ;-)
> 
> Ian Wadham wrote:
>     The comments on lines 295-298 explain why I have commented out the return statement. I am hoping a KDE core developer can suggest a better way of handling the situation than aborting Dr Konqi.

Well, the claim is that aborting for no cookie support is wrong in the first place.
If so, drop the code.
If not (or unsure) please don't "break" it with an unrelated patch.

- Thomas

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

On Juli 27, 2014, 9:16 vorm., Ian Wadham wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119498/
> -----------------------------------------------------------
> 
> (Updated Juli 27, 2014, 9:16 vorm.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Runtime, kdelibs, and Michael Pyne.
> 
> 
> Repository: kde-runtime
> 
> 
> Description
> -------
> 
> When a KDE app crashes in Apple OS X, it just disappears from the screen. At the most, the user is invited to report the crash to Apple. AFAIK this has been a problem in KDE on Apple OS X for years, leading to frustration with KDE among Apple users and MacPorts developers and an attitude among KDE developers of "Why does nobody report the problem(s) on bugs.kde.org?"
> 
> It is my strong belief that the failure to report crashes of KDE apps in Apple OS X also exists in Frameworks.
> 
> So far I have identified a number of portability bugs in KDE on Apple OS X: 1 in KCrash, 1 in kdeinit4 and 5 in Dr Konqi. Three patches for Dr Konqi are submitted in this review. Patches for KCrash and kdeinit4 are submitted in part 1 of this review, against kdelibs. I am still investigating the other two problems in Dr Konqi - and there could be more than two...
> 
> In this review we have three portability problems:
> 
> 1. On Apple OS X, Dr Konqi's dialog box hides itself underneath the main window of the app that has just crashed, so is effectively useless. This appears to be because Dr Konqi is started by a Linux/Unix method (fork() + exec()?). If an app is started with the Apple OS X "open" command, it always appears on top. The patch just raises the dialog box.
> 
> 2. When formatting the backtrace output, Dr Konqi crashes (with an ASSERT) on the last line. This appears to be an error in the algorithm used (i.e. also a bug in Linux KDE), but the patch is treating it as an Apple OS X portability problem for now.
> 
> 3. Dr Konqi checks whether the user can save cookies in kcookiejar and, if not, stops reporting the crash. On Apple OS X, cookies would be kept in another browser (e.g. Safari or Firefox) and not in KDE's default browser (Konqueror) and cookie jar. IMHO, Dr K should report the crash no matter what, as long as it can connect to bugs.kde.org and log in.
> 
> 
> Diffs
> -----
> 
>   drkonqi/reportassistantpages_bugzilla.cpp 86ca327 
>   drkonqi/gdbhighlighter.cpp 7cd0aa9 
>   drkonqi/main.cpp 75e060e 
> 
> Diff: https://git.reviewboard.kde.org/r/119498/diff/
> 
> 
> Testing
> -------
> 
> Using Apple OS X 10.7.5 (Lion) on a MacBook Pro, I have installed KDE libs via MacPorts (at version 4.12.5) and I have adapted kdesrc-build to run in an Apple OS X environment and used it to test against the KDE 4.13 branch. I have been testing with a KDE app that I can crash at will and using stderr and Apple OS X Console log output to determine the outcome.
> 
> Please note that I am the -only- KDE developer who has this kind of setup, but I am NOT a KDE core developer. My experience before now has been in KDE Games. However I used to be a UNIX and database guru before I retired in 1998.
> 
> I NEED HELP from KDE -core- developers to proceed further. These problems will also exist in Dr Konqi for KF 5, but I am as yet unable to build or test Frameworks on Apple OS X and I cannot find Dr Konqi among the Frameworks repositories. I am sure there are many more Apple OS X portability problems in Dr Konqi and other KDE software.
> 
> Without my patches, Dr Konqi, on Apple OS X, remains invisible to the user, often fails to complete the backtrace report and then fails to connect to bugs.kde.org.
> 
> With my patches, Dr Konqi on Apple OS X can generate a full crash report, including the backtrace and the results of the dialog with the user. Sometimes, however, it fails to submit the completed report to bugs.kde.org. This problem is still under investigation.
> 
> I would not have got this far without help from Michael Pyne, Thomas Lübking and several of the MacPorts developers, as well as the unfailing enthusiasm and encouragement of my friend Marko Käning.
> 
> 
> Thanks,
> 
> Ian Wadham
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20140729/8185382d/attachment-0001.html>