Ticket #192 (closed enhancement: fixed)

Opened 10 months ago

Last modified 7 months ago

Port to KDE 4

Reported by: diederik Owned by:
Priority: blocker Milestone: kmess-2.0
Component: Other Version: 1.5
Keywords: Cc:

Description (last modified by valerio) (diff)

We want to port KMess to KDE 4. This should be done first since all other tickets depend on it. I'll start a branch, and we can work though all modules in parallel.

General:

  • run standard qt3to4 porting script.
  • run adapt-to-kde4-api.pl script.
  • run am2cmake --kde4 to convert the build system to cmake
  • run uic3 -convert and fixuifiles to convert all UI files.

Port buildsystem:

  • check for package libxml
  • check for package libxslt
  • check for XScreenSaver extension library
  • use get-svn-version.sh again
  • add --enable-debug-messages
  • add autopackage checks for binreloc

Gui things:

  • update all icon names
  • use KXmlGuiMainWindow so all menus and toolbars become customizable (see techbase tutorials).
  • port the contact list to the model/view paradigm. See this page.

Protocol notes:

  • find out what networking classes to use.
  • QByteArray became much more powerful, even makes KMessBuffer obsolete.
  • QPtrList no longer does auto-delete, have to check for memory leaks.

Afterwards:

  • run cmakelint.pl
  • get rid of Qt3Support and Kde4Support classes by conversion scripts.
  • see if we can use KConfigXT
  • see what files to move for library-refactoring

More info:

Attachments

quick-changes.patch (6.9 kB) - added by valerio 10 months ago.
I had for a moment the illusion that these were the only files remaining to be ported, but i clashed on soapconnection and the consciousness there's still kmess and kmessinterface waiting to be ported. Fuck.
first run.png (61.8 kB) - added by valerio 10 months ago.
Ah, our first Qt4/KDE4 run. Such goodness!
kmess2-settingsdialog.png (325.2 kB) - added by diederik 10 months ago.
First run of the settings dialog, quite a funny view
kmess2-soaperror.png (0.8 MB) - added by diederik 10 months ago.
Current state of login, soap response is can't be read and after closing the dialog everything crashes

Change History

  Changed 10 months ago by diederik

  • description modified (diff)

  Changed 10 months ago by diederik

  • description modified (diff)

added build system notes

  Changed 10 months ago by diederik

  • description modified (diff)

  Changed 10 months ago by diederik

Note: there are extra porting scripts at:

http://websvn.kde.org/trunk/KDE/kdesdk/scripts/qt4/

  Changed 10 months ago by diederik

  • description modified (diff)

completed general part, ran all scripts to convert the sources automatically.

  Changed 10 months ago by diederik

  • description modified (diff)

  Changed 10 months ago by diederik

Notes:

  • QPtrList<Obj> becomes QList<Obj*> or QList<Obj>.
  • use foreach() macro to iterate.

Current way to start building is:

svn checkout http://kmess.svn.sourceforge.net/svnroot/kmess/branches/kmess/kde4porting/ kmess2
mkdir kmess2-build
cd kmess2-build
cmake ../kmess2
make

  Changed 10 months ago by diederik

Tips for porting QPtrList:

  • use foreach() for iterating
  • use qDeleteAll() to cleanup
  • use delete list_.take( i ) to remove an item, found before with int i = list_.indexOf( obj ); if needed.

QDict<Object> becomes QHash<String,Object*>

  Changed 10 months ago by diederik

Forgot: qDeleteAll() must be followed by a manual clear() call as well!

  Changed 10 months ago by diederik

  • description modified (diff)

mark XScreenSaver stuff as done, was commited earlier this day.

  Changed 10 months ago by valerio

  • description modified (diff)

follow-ups: ↓ 14 ↓ 15   Changed 10 months ago by valerio

I'm still undecided whether the use of QList<Obj> will be better than the use of QList<Obj*>.

I've found myself reconverting Chat Message Style's QLists to <Obj*> because of compatibility with current code (r2409) and added a foreach() in the destructor there to clean up; but I'm thinking that maybe directly using the objects can be better - not to mention we wouldn't have to manually destroy them :)

in reply to: ↑ 13   Changed 10 months ago by valerio

Replying to valerio:

I'm still undecided whether the use of QList<Obj> will be better than the use of QList<Obj*>. I've found myself reconverting Chat Message Style's QLists to <Obj*> because of compatibility with current code (r2409) and added a foreach() in the destructor there to clean up; but I'm thinking that maybe directly using the objects can be better - not to mention we wouldn't have to manually destroy them :)

Uh, sorry, I got a bit confused. I've reconverted Chat View's QLists and cleaned them up in Chat View's destructor. The rev is r2410.

in reply to: ↑ 13   Changed 10 months ago by diederik

Replying to valerio:

I'm still undecided whether the use of QList<Obj> will be better than the use of QList<Obj*>. I've found myself reconverting Chat Message Style's QLists to <Obj*> because of compatibility with current code (r2409) and added a foreach() in the destructor there to clean up; but I'm thinking that maybe directly using the objects can be better - not to mention we wouldn't have to manually destroy them :)

You'll have problems if you want to share the objects. Each time you try to access an object you'll get a copy. I actually had a hard time fixing those issues. Keeping the object as pointer also removes the need to have the include for "Obj" in your .h file, reducing the number of dependencies and files that have to be recompiled if you change "obj".

  Changed 10 months ago by diederik

Thing to watch out for: QTimer::start( int time, bool singleShot ) no longer has the singleShot argument. You'll have to call QTimer::setSingleShot( true ) or the timer will be called multiple times by default.

  Changed 10 months ago by diederik

Btw I noticed your cast to static_cast<QWidget *>( khtmlPlaceholder_ ) in r2410. I've haven't fixed the compilation issues with GUI-classes yet. So MsnNotificationConnection and Current Account won't compile yet until KMess inherits from QWidget again. Actually those classes shouldn't refer to the GUI anymore, but that's a different story.

  Changed 10 months ago by valerio

I was using pointers already, fortunately :) Thanks for removing that doubt!

I knew about that QTimer issue, i had a compilation error on a QTimer and noticed the API change.

About casts, I have Current Account perfectly compiling here, and neither it nor Account do inherit from QWidget..

Another issue I've been dealing pretty much everywhere. Configuration. It's a crappy system or it is just me?! Many read/saveProperties() methods take KConfigGroup parameters, so am I supposed to call, before the read/save method: KConfigGroup group = KGlobal::config()->group( "Chat Window" ); ?!!? And what the hell happens when KMainWindow calls them? What group does it give to those methods?? I still haven't finished porting Chat Window mainly due to this issue...

follow-up: ↓ 20   Changed 10 months ago by diederik

Cool to see more of the UI fixed in trac today, great work! :D

Another issue I've been dealing pretty much everywhere. Configuration. It's a crappy system or it is just me?! Many read/saveProperties() methods take KConfigGroup parameters, so am I supposed to call, before the read/save method:

Yeah I have a hard time over the KConfig stuff too, especially stuff like

ChatWindow::readProperties( const KConfigGroup & )

I'm not sure yet either either how that is supposed to work. Perhaps you'll have to assign the group somewhere, since we currently desided that on our own. It's also possible the KConfigGroup points to the global group, so you can create a "subgroup" in there which stored the real settings.

So far my findings:

They revised the API to make it more logical. In a certain way it is. In KDE 3 you called setGroup(..) all the time which switched the "current group" of the KConfig object. Now you request the group object first so you don't change the global object.

It seams you have a central KConfig object now. Calling KConfig globalConfig; gives you that object by default. You can also have groups inside groups and iterate easily over those, which is much easier then our current broken "Contact_accountHandle_contactHandle", but I didn't change that yet for backwards-compatibility.

About casts, I have CurrentAccount perfectly compiling here, and neither it nor Account do inherit from QWidget..

Nope I actually meant, would it still compile if you removed the cast now? It didn't compile previously off course because KMess didn't inherit from QWidget because we have to do that manually now.

And what the hell happens when KMainWindow calls them? What group does it give to those methods?? I still haven't finished porting ChatWindow mainly due to this issue...

I think you can find the name of the section in kmessrc.

I still haven't finished porting ChatWindow mainly due to this issue...

Well it's also blocking MsnNotificationConnection, which shouldn't bother with configs anymore actually but I didn't want to revise that while porting. Currently I work a bit arround this issue so we can look at this globally after all other mess is sorted out.

in reply to: ↑ 19   Changed 10 months ago by valerio

Replying to diederik:

Cool to see more of the UI fixed in trac today, great work! :D

Thanks :D

You can also have groups inside groups and iterate easily over those, which is much easier then our current broken "Contact_accountHandle_contactHandle", but I didn't change that yet for backwards-compatibility.

I saw a KConfig*something class made just for that, we'll see after the port.

Nope I actually meant, would it still compile if you removed the cast now? It didn't compile previously off course because KMess didn't inherit from QWidget because we have to do that manually now.

*that* specific cast wouldn't work because KHtmlPlaceholder_ is in the UI file and is actually a QFrame - so it's a widget, just Chat Message View won't accept it in QFrame form; I can't see at the moment which problems do Current Account and Msn Switchboard Connection have without a QWidget-inherited KMess... will do later probably, I'm porting some root folder files atm.

I think you can find the name of the section in kmessrc.

I hope so, let's bring kmess back to life, then I'll search for more info.

I still haven't finished porting ChatWindow mainly due to this issue...

Well it's also blocking MsnNotificationConnection, which shouldn't bother with configs anymore actually but I didn't want to revise that while porting. Currently I work a bit arround this issue so we can look at this globally after all other mess is sorted out.

I've solved it just not caring :D I just use whatever group the app sends to me; when I do have to call the read/saveProperties methods, for now I've made it to manually select the "Chat Window" group. It's probably wrong, but hey, it's compiling!

  Changed 10 months ago by valerio

I'm having an hard time porting the big pile of hacks&workarounds we had to put in place for the Contact List UI classes. For example, I've temporarily changed the contactlist items classes to use QTreeWidget, for porting simplicity; but it's actually impossible to see if it's the right choice since we can't see the results. We will probably be better off using QTreeView and implementing a custom QAbstractItemView.

I also had to comment out heavily KMessListViewItem, because we've had a workaround to display rich text in its items, and we'll have to wait for the custom QAbstractItemView.

My hopes that the last porting commit would have made kmess to get back to life have all vanished :(

Changed 10 months ago by valerio

I had for a moment the illusion that these were the only files remaining to be ported, but i clashed on soapconnection and the consciousness there's still kmess and kmessinterface waiting to be ported. Fuck.

follow-up: ↓ 23   Changed 10 months ago by diederik

  • description modified (diff)

Cool you've managed to work through network/upnp too. :-) you've committed a lot again :-D

I've temporarily changed the contactlist items classes to use QTreeWidget, for porting simplicity; but it's actually impossible to see if it's the right choice since we can't see the results.

As for the user interface, yes we'll have to wait and see.. can't do much about it. :-( I guess commenting out is a better option to make it compile again.

We will probably be better off using QTreeView and implementing a custom QAbstractItemView.

Qt4 offers a nice model/view classes to use, which makes the views far more powerful. We should consider porting it to that later.

--- Some other things I noticed in your comments:

There is a memory leak now in NotificationManager. The qDeleteAll() does remove all remaining stacks when the class is terminated, but you don't delete the object at the remove() call (since there is no auto-delete anymore). You'll find an example in my class to fix that, it's something like:

int i = stacks_.indexOf( item );
delete stacks_.takeAt( i );

Also two small points we can fix globally afterwards:

  • remove kdebug.h everywhere. it's imported by kmessdebug.h on purpose.
  • remove kdeversion.h everywhere.
  • rewrite the remaining .h include lines if they are still needed. Fewer includes means faster compiling.

One small thing, you may want to consider replacing this with a foreach():

for( it = emoticonTheme_->begin(); it != emoticonTheme_->end(); ++it )
{
  emoticon = *it;

like:

foreach( emoticon, emoticonTheme_ )
{

or:

foreach( Emoticon *emoticon, emoticonTheme_ )
{

Likewise,

for( int x = 0; x < themeDirs.count(); x++ )

becomes:

foreach( const QString &dirName, themeDirs )

Thankfully we can finally have a consistent loop syntax in Qt4. :D

The filename changes for .ui files are also cool. Does that mean we can get rid of all those "Interface" suffixes, and name all UI files the same as their .cpp counterpart?

in reply to: ↑ 22   Changed 10 months ago by valerio

Replying to diederik:

Cool you've managed to work through network/upnp too. :-) you've committed a lot again :-D

And right on time, since in 1 or at most, two days, i've got to start another studying rush, and it'll be looong.

There is a memory leak now in NotificationManager. The qDeleteAll() does remove all remaining stacks when the class is terminated, but you don't delete the object at the remove() call (since there is no auto-delete anymore). You'll find an example in my class to fix that, it's something like: {{{ int i = stacks_.indexOf( item ); delete stacks_.takeAt( i ); }}}

Done :D There will probably be more, though.

Also two small points we can fix globally afterwards: - remove kdebug.h everywhere. it's imported by kmessdebug.h on purpose. - remove kdeversion.h everywhere. - rewrite the remaining .h include lines if they are still needed. Fewer includes means faster compiling.

True, there are a whole lot of file with old, unused and even duplicated inclusions.

One small thing, you may want to consider replacing this with a foreach(): {{{ for( it = emoticonTheme_->begin(); it != emoticonTheme_->end(); ++it ) { emoticon = *it; }}}

Tried. It didn't work. It's because Emoticon Theme lacks the operator, and I didn't want to add it while porting.

Likewise, {{{ for( int x = 0; x < themeDirs.count(); x++ ) }}} becomes: {{{ foreach( const QString &dirName, themeDirs ) }}}

Oh, that just slipped past :)

The filename changes for .ui files are also cool. Does that mean we can get rid of all those "Interface" suffixes, and name all UI files the same as their .cpp counterpart?

I hope so! Also, considering the UI classes are on a namespace on their own, we can even call the UI widgets the same as the class which will implement them :o

--

Oh, and: great news! KMess is compiling atm! ...just, it's not linking it's got a whole bunch of unexported calls. I'm trying to fix them now.

  Changed 10 months ago by valerio

I've removed in my copy *all* Qt3 and KDE3 inclusions, and switched all the remaining points where they were being used to Qt4/KDE4 equivalents. I'm currently committing all those changes, they're a whole lot :P

Changed 10 months ago by valerio

Ah, our first Qt4/KDE4 run. Such goodness!

  Changed 10 months ago by diederik

  • status changed from new to assigned

Dude you rock!

  Changed 10 months ago by diederik

  • owner diederik deleted
  • status changed from assigned to new

I feel we should also do a first screenshot when it actually manages to render a UI too :P

  Changed 10 months ago by diederik

I've managed to get past the first crash and endless-loop and things are starting up. Currently the QHttp object doesn't return correct values for the login.

I'm getting strange crashes, in the settings dialog browseButton_ is at the memory address 0x03 according to gdb. After the http error, a call to KMess::disconnect() claims the object is at 0x01. So far I haven't debugged stuff in kdevelop. This might help: http://sourceware.org/gdb/current/onlinedocs/gdb_7.html#SEC47

Basically:

gdb ./kmess/kmess
run
...on crash reports
bt
frame 1  # to switch frames
info locals  # to display local vars
quit

Also worth mentioning: running qhash.find( key ).value() returned 1 for me once, which becomes 0x01 if assigned to a pointer and the key does not exist. It's recommended to use qhash.value( key, 0 ) instead.

  Changed 10 months ago by diederik

Q_ASSERT seams to fall qFatal() which aborts the application. We'll have to find a way to fix the asserts later. E.g. define our own KMESS_ASSERT.

  Changed 10 months ago by diederik

Another bug: timeouts keep running if the passport login service has a SOAP error.

  Changed 10 months ago by diederik

Btw, merging this branch with trunk would totally suck if we do a lot of file renames or moves, so I'd delay those things for now.

  Changed 10 months ago by diederik

Yay the settings dialog opens again if I disable the loading the emoticon settings.

The other weird crashes are fixed for this one. The setupUi() calls shouldn't initialize itself, but a QWidget. To make this move obvious, I've made separate ..Page classes for these widgets and moved setupUi() in there. Seams more clean to me since that function is inline, so it's code is inserted at the place it's called.

Attached is a screenshot of the current state. I guess we've messed up our parent widget values.. :P

Changed 10 months ago by diederik

First run of the settings dialog, quite a funny view

Changed 10 months ago by diederik

Current state of login, soap response is can't be read and after closing the dialog everything crashes

  Changed 10 months ago by diederik

I still get crashes in QHttp which seriously hinders the ability to test the network code.

In the meantime, I've improved KMessTest so you can run different tests like:

./kmess/kmess --runtest chatwindow

Or if you want to run it under gdb directly, try:

gdb -ex run --args ./kmess/kmess --runtest chatwindow

  Changed 10 months ago by diederik

We're able to connect now. Network code seams mostly ok except for some glitches. Most work will be the remaining UI.

  Changed 10 months ago by valerio

The chat window crash is due to the ChatView. If you start a chatwindow test (great feature!!!) and don't type anything, then close the window, kmess exits normally. But try again sending a message, and it'll crash.

I've also found a bug in the changes I've made to the contact list widgets loading logic. I probably forgot to reset one of the pointers to zero.

I didn't have time to check these out yet.

  Changed 9 months ago by diederik

  • description modified (diff)

Awesome you fixed the chatwindow crash! It seams it's getting usable enough to start actual testing and re-implement the missing parts :)

  Changed 9 months ago by valerio

eheh :)

Actually i did also tried to start looking around for the Contact List redesign. It is going to be a great challenge: but I'm not really certain about how to implement it. Using Qt4 MVC paradigms we can easily create a model for our Contacts and Groups. We could even get to integrate the entire ContactList class with a Qt4 model, by subclassing an abstract model instead of QObject.

The issue is, we would have to change much code depending on how far we go in placing models inside the contactlist part... What do you think?

  Changed 9 months ago by valerio

  • description modified (diff)

  Changed 9 months ago by valerio

  • description modified (diff)

  Changed 9 months ago by valerio

  • description modified (diff)

  Changed 9 months ago by valerio

  • description modified (diff)

I've caught two more bugs with file transfers. When KMess offers to become the server, it tries to bind itself to a port, but fails:

kmess(26307) ApplicationList::addServerConnection: adding new server socket.
kmess(26307) DirectConnectionBase::getLocalServerPort: Reserved port  6891  for server socket.
kmess(26307) DirectConnectionPool::addServerConnection: Adding new server connection to listen at port 6891
kmess(26307) DirectConnectionBase::openServerPort: Creating server socket.
kmess(26307) DirectConnectionBase::openServerPort: Could not listen at port  6891 , freeing for next one.

The error reported by QTcpServer::serverError() is QAbstractSocket::Unknown Socket Error - binding to all available ports fails, so the transfer is conducted via the SB.

I've tried to extend the range of ports above the 6900, but with no luck. The #qt folks couldn't help me, neither. I've compiled a very little test app which just tries binding to the same ports in the exact same way as KMess does, but it succeeds at first try (port 6891).

The second bug happens when starting a file transfer from KMess2:

WARNING: kmess(15117) P2PApplication::sendCancelMessage: aborting while INVITE ack was not received (state= 18  contact= "contact@kmess.org"  session= 1279552  class= FileTransferP2P  action=continue,sendslpbye). 
WARNING: ASSERT: "waitingState_ == P2P_WAIT_FOR_SLP_BYE_ACK" in file ~/kmess/2.0/kmess/network/applications/p2papplication.cpp, line 865
WARNING: kmess(15117) P2PApplication::gotTransferAbortedAck: Got an unexpected P2P transfer aborted-response (flag= "80"  state= 2  contact= "contact@kmess.org"  session= 2310892  class= FileTransferP2P  action=endapplication). 
WARNING: kmess(15117) P2PApplication::gotMessage: received another message while awaiting termination (flags=0x "0"  size= 361  contact= "contact@kmess.org"  session= 2310892  class= FileTransferP2P  action=unsetclosing). 
WARNING: kmess(15117) P2PApplication::gotSlpBye: Received BYE to abort when all data is already transferred (state= 17  contact= "contact@kmess.org"  session= 2310892  class= FileTransferP2P  action=endapplicationlater) 

These are possibly the last network-related bugs.


Apart from those, porting is almost over. The network code works without issues except those two bugs explained above. All the GUI code is okay too, with the great exception of the contact list, which was too hacky to be just ported without effort. See Contact List Model for my effort at replacing the old contact list.

I'm committing a couple more fixes to very small issues. After all the fixes explained in this comment, we should be set and ready to end the porting process: then we'll be able to actually start working on some 2.0 tickets ;D

  Changed 9 months ago by richard

  • description modified (diff)

I think I just got the last non-kde4 icons out (excluding emoticons) The icons for contacts status i.e. online, busy, offline, now use oxygen standard icons and overlays

  Changed 8 months ago by valerio

  • description modified (diff)

  Changed 8 months ago by valerio

Forgot to comment the description change...

I've added some more checks to the CMake config file; you can use the CMake option "-D KMESS_DEBUG_OUTPUT=1" to enable debug output on release builds; on debugfull build type, it's enabled by default.

I've also fixed the #define for XScreensaver, so if it's not present, the screensaver/xautolock code gets #ifdef'ed away.

  Changed 7 months ago by diederik

Ugh this is getting long, but posting here anyway.

So far I can spot the following bugs:

  • Model is reset when a contact status changes
  • Picture browse button doesn't correctly resize/crop the picture.
  • When a font color is selected in the chat window it's reset after opening the account settings.
  • Chat window font dialog does not have the current value selected.
  • Emoticon theme after saving the account settings, because the current one is not selected.
  • Away/busy are not mixed, though I guess this is intentional because the Oxygen icons differ too much?
  • Resuming a chat does not work. The contact remains gray.

  Changed 7 months ago by diederik

  • status changed from new to closed
  • resolution set to fixed

The porting is done, so creating a new ticket instead to fix the remaining regressions: #210.

This topic is really getting too long ;-)

Note: See TracTickets for help on using tickets.