> 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 > >
