Review Request 119498: Report crashes of KDE apps in Apple OS X (2) (fix drkonqi)
Ian Wadham
iandw.au at gmail.com
Tue Sep 16 12:05:16 BST 2014
> On July 27, 2014, 11:17 a.m., Thomas Lübking wrote:
> >
This part of the patch, to fix the bug in the backtrace lines highlighter, has been re-written as suggested by Thomas, by adding a dummy entry to the lines, and the ASSERT remains because we should never get past the dummy, however many '\n' characters are in the last entry in the backtrace.
> On July 27, 2014, 11:17 a.m., 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.
>
> Thomas Lübking wrote:
> You can omit the system test.
> It's ok to call "raise()" here on any system.
For safety, raise() will be used on all platforms and window managers, making sure the Dr Konqi dialog is always visible.
> On July 27, 2014, 11:17 a.m., 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).
>
> René J.V. 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 :)
>
> Thomas Lübking wrote:
> 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.
The cookie-checking code will be bypassed on Apple OS X for now, because it can cause Dr Konqi to crash. It is intended that it will be replaced by code to handle the new token-checking policy of Bugzilla 4.4.3 and later (4.4.5 on bugs.kde.org).
> On July 27, 2014, 11:17 a.m., 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.
>
> Thomas Lübking wrote:
> 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.
>
> Aaron J. Seigo wrote:
> The login relies on cookies so the check must stAy. The check should remain osx in case in future this works. However what should probably happen is the Login button and text fields should be disabled pendiNg a call to see if cookies are enabled. That check should made async and the message boxes removed in favour oof inline messages Incliding A link to tirn cookies on instead of a yes/no dialog.
> Sorry for the typos. This form barel works at all with the android web browser :(
> miAnimizin
>
> Ian Wadham wrote:
> The login works OK, cookies or not. Dr K asks you to enter user name and password every time, regardless of whether you are already logged in and have cookies. In my case, the cookies are in Firefox in whatever location it uses with Apple OS X. KDE cannot find them there, but maybe Qt can or will be able to. I have tried (briefly) going the KDE way, using Konqueror and defining it (to KDE preferences) as my preferred browser and then logging in to BKO with Konqueror, but on Apple OS X that is cumbersome to set up and does not work predictably in Dr K, as it must if we want the average user to report bugs.
>
> After the login, it was possible to report a bug completely https://bugs.kde.org/show_bug.cgi?id=336075#c3 on BKO, but with the cookie check enabled one cannot report anything, because Dr K crashes in the middle of the cookie check (on Apple OS X).
>
> Dr K uses remote procedure calls to do its work on BKO (class KXmlRpc::Client), including logging in. See code in kde-runtime/drkonqi/bugzillalib.cpp, lines 68-210. It would be nice if Dr K could somehow connect to BKO via the user's browser of choice or cookies therein, using something really portable, similar to QDesktopServices::openUrl(), but I cannot see any immediate way to do that.
>
> I have another bug, not yet investigated in detail, where Dr K has lost the login to BKO somehow and cannot submit the completed report, but it was still able to save it in a file.
>
> It is hard to know how to test Dr K's bug-report submission thoroughly without cluttering up BKO with irrelevant reports. Is there a test version of the BKO database somewhere?
>
> Ben Cooksley wrote:
> The use of the Bugzilla XMLRPC api is required i'm afraid. The web browser interface cannot be used - due to CSRF protections now built into it. In addition, it changes from time to time breaking compatability. This form of HTML scraping was used by prior versions of Dr Konqi - and broke quite badly when Bugzilla was upgraded to 4.2.x.
>
> Ian Wadham wrote:
> @Ben Cooksley: Understood. I was thinking more along the lines of connecting without starting a browser window (which would be a nuisance for the user anyway), but somehow using the cookies that might have been stored by the user's browser of choice, and if that fails THEN ask for username and password. Our problem, on Apple OS X, is that none of Dr K works ATM if your browser of choice is Safari or Firefox or even Konqueror, because the cookies are stored somewhere that is not the KCookieJar. Maybe the Qt guys solved this problem somewhere, e.g. when developing QDesktopServices::openUrl(). Of course, Dr K must still use KXmlRpc::Client for data transfer to and fro.
>
> René J.V. Bertin wrote:
> ? Maybe it's because I have Chrome as my default browser, but Konqueror uses the kcookiejar over here. If I make sure kded4 is running, that is; otherwise it can't.
> I do have a bit of a hard time believing that someone tweaked things so that KDE would use the native cookie store but as a function of which browser you have configured as a default. (If I'd have to chose I'd say that support for native plugins is a tad more useful!)
>
> Ben Cooksley wrote:
> Unfortunately Bugzilla has removed support for using cookies to submit XMLRPC API queries in version 4.4, in favour of access tokens so it isn't possible to do that either i'm afraid :(
>
> Ian Wadham wrote:
> It seems that bugs.kde.org has changed to using Bugzilla 4.4.5 in recent days, but I see no corresponding changes for Dr Konqi source code to use tokens. So what is going on?
>
> Also it looks as though RPC for WebServices User.login is deprecated in Bugzilla 5.0, all of it (see http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html) and that tokens will in turn give way to "Bugzilla_api_key" or supplying login name and password with every remote procedure call (see http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN).
>
> I might have a try at using tokens in Dr K (conditional on Q_OS_MAC) and see if that will work when no cookies are visible. What do you think?
>
> René J.V. Bertin wrote:
> I think you'd better ascertain first that BKO will stick to unmodified and uptodate Bugzilla first ;)
>
> Ben Cooksley wrote:
> We will be doing as much as possible to reduce the number of modifications - and staying up to date.
>
> As it seems Bugzilla upstream has no problem with regular changes in their API, we may need to put in place a shim which takes in certain requests needed by Dr Konqi for it to work - and then communicates with Bugzilla to do the necessary actions. Volunteers?
>
> Ian Wadham wrote:
> It looks as though bugzillalib.cpp, part of Dr K, is already fairly thin. What it needs, perhaps, is a check of the Bugzilla version, backed by alternative compositions of the parameter maps and interpretations of responses from BKO, depending on whether we are still using "this" version of Bugzilla or "the next". Perhaps the changeover to a new version of BKO could then be coded for in Dr K and released in KDE software ahead of time, whenever convenient for the KDE community and downstream people.
>
> I am not volunteering, though. KCrash and Dr K are currently not working at all on Apple OS X. My objective is to fix that, by any means whatever, and move on to the next KDE portability problem on Apple OS X. I have been stuck on this one for far too long already (about 2 months) and have still not reached the end of the Dr Konqi problems.
>
> See my comments under "Testing Done" above.
I am working on code to handle tokens instead of cookies. This should work on all platforms.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119498/#review63254
-----------------------------------------------------------
On Sept. 16, 2014, 10:49 a.m., Ian Wadham wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119498/
> -----------------------------------------------------------
>
> (Updated Sept. 16, 2014, 10:49 a.m.)
>
>
> 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/gdbhighlighter.cpp 7cd0aa9
> drkonqi/main.cpp 75e060e
> drkonqi/reportassistantpages_bugzilla.cpp 86ca327
>
> 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.
>
>
> File Attachments
> ----------------
>
> Log of Dr K ASSERT problem
> https://git.reviewboard.kde.org/media/uploaded/files/2014/07/30/a3f99f00-94df-4b10-bc47-66b1c966f893__DrKonqiASSERT.kcrash.txt
>
>
> Thanks,
>
> Ian Wadham
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20140916/718d557a/attachment.htm>
More information about the kde-core-devel
mailing list