From kde-core-devel Mon Oct 21 16:48:11 2002 From: Lubos Lunak Date: Mon, 21 Oct 2002 16:48:11 +0000 To: kde-core-devel Subject: Re: [Bug 49369] Store Window Settings doesn't work with KMail X-MARC-Message: https://marc.info/?l=kde-core-devel&m=103521890818862 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--------------Boundary-00=_B0CCERIVPUK5O19OWCFM" --------------Boundary-00=_B0CCERIVPUK5O19OWCFM Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 8bit On Friday 18 October 2002 23:47, Ingo Klöcker wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Hi *whoever implemented this "Store Window Settings" feature*, > > please tell us, the KMail developers, how we can fix this bug. And > please keep in mind that KMail currently has two different main > windows. The composer window and the "main window" (with folder, header > and preview pane and optionally mimetree viewer). > > How does KWin differentiate between the composer window and the main > window when it stores and restores window settings? Actually, I started looking at this issue at the end of the last week. The only suitable window properties for identifying windows AFAIK are WM_NAME, WM_CLASS and WM_WINDOW_ROLE (see http://mail.gnome.org/archives/usability/2002-March/msg00012.html). Using the window caption (WM_NAME) might probably cause more harm than good, so that leaves only WM_CLASS and WM_WINDOW_ROLE. In Konsole, use " xprop | grep '\(CLASS\)\|\(ROLE\)' " and click on various windows (Qt now incorrectly uses WINDOW_ROLE without the WM_ prefix, see qapplication_x11.cpp.patch). If the output is the same, the windows will be considered the same. I said 'will be', after applying the attached kdebase/kwin patch, feel free to review (oh, in case somebody notices the removal of SessionInfoPrivate, that's a private class anyway). For KMail (and all other apps): The WM_CLASS value is basically argv[0], so you cannot do much about it. The WM_WINDOW_ROLE is set by Qt to the value of QWidget::name() (i.e. the const char* argument to QWidget constructor, or QWidget::setName()). I suggest setting it explicitly for all toplevel widgets (different values for different types of widgets of course). As a slight short-term disadvantage, this may break some DCOP scripts *shrug* hardcoding kmail-mainwindow#1 somewhere may sometimes break even now. Hmm, and while I'm at it, I also suggest the kmainwindow.cpp patch which makes sure the name() will be really always unique. Probably not really needed if I get in Qt a similar patch for QWidget::setName() for toplevel widgets. PS: I reassigned the bug to kwin, as it's a kwin bug (fixed by the attached patch). Just set name() for toplevel widgets in KMail as I said (unless somebody will complain about the possible DCOP breakage or whatever). -- Lubos Lunak KDE developer --------------------------------------------------------------------- SuSE CR, s.r.o. e-mail: l.lunak@suse.cz , l.lunak@kde.org Drahobejlova 27 tel: +420 2 9654 2373 190 00 Praha 9 fax: +420 2 9654 2374 Czech Republic http://www.suse.cz/ --------------Boundary-00=_B0CCERIVPUK5O19OWCFM Content-Type: text/x-diff; charset="iso-8859-1"; name="qapplication_x11.cpp.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="qapplication_x11.cpp.patch" --- qapplication_x11.cpp.sav Fri Oct 18 11:05:55 2002 +++ qapplication_x11.cpp Fri Oct 18 15:14:30 2002 @@ -1745,7 +1745,7 @@ void qt_init_internal( int *argcptr, cha qt_x11_intern_atom( "WM_STATE", &qt_wm_state ); qt_x11_intern_atom( "WM_TAKE_FOCUS", &qt_wm_take_focus ); qt_x11_intern_atom( "WM_CLIENT_LEADER", &qt_wm_client_leader); - qt_x11_intern_atom( "WINDOW_ROLE", &qt_window_role); + qt_x11_intern_atom( "WM_WINDOW_ROLE", &qt_window_role); qt_x11_intern_atom( "SM_CLIENT_ID", &qt_sm_client_id); qt_x11_intern_atom( "CLIPBOARD", &qt_xa_clipboard ); qt_x11_intern_atom( "RESOURCE_MANAGER", &qt_resource_manager ); --------------Boundary-00=_B0CCERIVPUK5O19OWCFM Content-Type: text/x-diff; charset="iso-8859-1"; name="kwin.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="kwin.patch" --- client.cpp.sav Fri Oct 18 15:23:54 2002 +++ client.cpp Mon Oct 21 17:00:21 2002 @@ -95,6 +95,7 @@ class ClientPrivate { public: ClientPrivate() {}; + QCString windowRole; }; }; @@ -118,6 +119,8 @@ static bool blockAnimation = FALSE; static QRect* visible_bound = 0; +static QCString getStringProperty(WId w, Atom prop, char separator=0); + void Client::drawbound( const QRect& geom ) { if ( visible_bound ) @@ -570,6 +573,7 @@ Client::Client( Workspace *ws, WId w, QW getWmNormalHints(); // get xSizeHint getWmClientLeader(); fetchName(); + d->windowRole = getStringProperty( w, qt_window_role ); if ( mainClient()->isSticky() ) setSticky( TRUE ); @@ -1355,6 +1359,8 @@ bool Client::propertyNotify( XPropertyEv getWindowProtocols(); else if (e.atom == atoms->wm_client_leader ) getWmClientLeader(); + else if( e.atom == qt_window_role ) + d->windowRole = getStringProperty( win, qt_window_role ); break; } return TRUE; @@ -2827,7 +2833,7 @@ void Client::keyPressEvent( uint key_cod QCursor::setPos( pos ); } -static QCString getStringProperty(WId w, Atom prop, char separator=0) +static QCString getStringProperty(WId w, Atom prop, char separator) { Atom type; int format, status; @@ -2936,7 +2942,7 @@ void Client::getWmClientLeader() */ QCString Client::windowRole() { - return staticWindowRole(win); + return d->windowRole; } /*! --- workspace.cpp.sav Fri Oct 18 13:26:54 2002 +++ workspace.cpp Mon Oct 21 17:04:28 2002 @@ -3706,6 +3706,7 @@ void Workspace::storeSession( KConfig* c config->writeEntry( QString("staysOnTop")+n, c->staysOnTop() ); config->writeEntry( QString("skipTaskbar")+n, c->skipTaskbar() ); config->writeEntry( QString("skipPager")+n, c->skipPager() ); + config->writeEntry( QString("windowType")+n, windowTypeToTxt( c->windowType())); } config->writeEntry( "count", count ); } @@ -3742,6 +3743,7 @@ void Workspace::loadSessionInfo() info->staysOnTop = config->readBoolEntry( QString("staysOnTop")+n, FALSE ); info->skipTaskbar = config->readBoolEntry( QString("skipTaskbar")+n, FALSE ); info->skipPager = config->readBoolEntry( QString("skipPager")+n, FALSE ); + info->windowType = txtToWindowType( config->readEntry( QString("windowType")+n ).latin1()); } } @@ -3755,6 +3757,7 @@ void Workspace::loadFakeSessionInfo() QString n = QString::number(i); SessionInfo* info = new SessionInfo; fakeSession.append( info ); + info->windowRole = config->readEntry( QString("windowRole")+n ).latin1(); info->resourceName = config->readEntry( QString("resourceName")+n ).latin1(); info->resourceClass = config->readEntry( QString("resourceClass")+n ).latin1(); info->wmClientMachine = config->readEntry( QString("clientMachine")+n ).latin1(); @@ -3768,6 +3771,7 @@ void Workspace::loadFakeSessionInfo() info->staysOnTop = config->readBoolEntry( QString("staysOnTop")+n, FALSE ); info->skipTaskbar = config->readBoolEntry( QString("skipTaskbar")+n, FALSE ); info->skipPager = config->readBoolEntry( QString("skipPager")+n, FALSE ); + info->windowType = txtToWindowType( config->readEntry( QString("windowType")+n ).latin1()); } } @@ -3777,6 +3781,7 @@ void Workspace::storeFakeSessionInfo( Cl return; SessionInfo* info = new SessionInfo; fakeSession.append( info ); + info->windowRole = c->windowRole(); info->resourceName = c->resourceName(); info->resourceClass = c->resourceClass(); info->wmClientMachine = c->wmClientMachine(); @@ -3790,6 +3795,7 @@ void Workspace::storeFakeSessionInfo( Cl info->staysOnTop = c->staysOnTop(); info->skipTaskbar = c->skipTaskbar(); info->skipPager = c->skipPager(); + info->windowType = c->windowType(); } void Workspace::writeFakeSessionInfo() @@ -3800,6 +3806,7 @@ void Workspace::writeFakeSessionInfo() for ( SessionInfo* info = fakeSession.first(); info; info = fakeSession.next() ) { count++; QString n = QString::number(count); + config->writeEntry( QString("windowRole")+n, info->windowRole.data() ); config->writeEntry( QString("resourceName")+n, info->resourceName.data() ); config->writeEntry( QString("resourceClass")+n, info->resourceClass.data() ); config->writeEntry( QString("clientMachine")+n, info->wmClientMachine.data() ); @@ -3813,6 +3820,7 @@ void Workspace::writeFakeSessionInfo() config->writeEntry( QString("staysOnTop")+n, info->staysOnTop ); config->writeEntry( QString("skipTaskbar")+n, info->skipTaskbar ); config->writeEntry( QString("skipPager")+n, info->skipPager ); + config->writeEntry( QString("windowType")+n, windowTypeToTxt( info->windowType )); } config->writeEntry( "count", count ); } @@ -3843,7 +3851,7 @@ SessionInfo* Workspace::takeSessionInfo( if (! sessionId.isEmpty() ) { // look for a real session managed client (algorithm suggested by ICCCM) for (SessionInfo* info = session.first(); info && !realInfo; info = session.next() ) - if ( info->sessionId == sessionId ) { + if ( info->sessionId == sessionId && sessionInfoWindowTypeMatch( c, info )) { if ( ! windowRole.isEmpty() ) { if ( info->windowRole == windowRole ) realInfo = session.take(); @@ -3859,7 +3867,8 @@ SessionInfo* Workspace::takeSessionInfo( for (SessionInfo* info = session.first(); info && !realInfo; info = session.next() ) if ( info->resourceName == resourceName && info->resourceClass == resourceClass && - info->wmClientMachine == wmClientMachine ) + info->wmClientMachine == wmClientMachine && + sessionInfoWindowTypeMatch( c, info )) if ( wmCommand.isEmpty() || info->wmCommand == wmCommand ) realInfo = session.take(); } @@ -3868,7 +3877,8 @@ SessionInfo* Workspace::takeSessionInfo( for (SessionInfo* info = fakeSession.first(); info && !fakeInfo; info = fakeSession.next() ) if ( info->resourceName == resourceName && info->resourceClass == resourceClass && - info->wmClientMachine == wmClientMachine ) + ( windowRole.isEmpty() || windowRole == info->windowRole ) && + sessionInfoWindowTypeMatch( c, info )) fakeInfo = fakeSession.take(); // Reconciliate @@ -3883,6 +3893,38 @@ SessionInfo* Workspace::takeSessionInfo( return 0; } +bool Workspace::sessionInfoWindowTypeMatch( Client* c, SessionInfo* info ) +{ + if( info->windowType == -2 ) { // undefined (not really part of NET::WindowType) + return c->windowType() == NET::Unknown || c->windowType() == NET::Normal + || c->windowType() == NET::Dialog || c->windowType() == NET::Override; + } + return info->windowType == c->windowType(); +} + +static const char* const window_type_names[] = { + "Unknown", "Normal" , "Desktop", "Dock", "Toolbar", "Menu", "Dialog", + "Override", "TopMenu" }; + +const char* Workspace::windowTypeToTxt( NET::WindowType type ) +{ + if( type >= NET::Unknown && type <= NET::TopMenu ) + return window_type_names[ type + 1 ]; // +1 (unknown==-1) + if( type == -2 ) // undefined (not really part of NET::WindowType) + return "Undefined"; + kdFatal() << "Unknown Window Type" << endl; + return NULL; +} + +NET::WindowType Workspace::txtToWindowType( const char* txt ) +{ + for( int i = NET::Unknown; + i <= NET::TopMenu; + ++i ) + if( qstrcmp( txt, window_type_names[ i + 1 ] ) == 0 ) // +1 + return static_cast< NET::WindowType >( i ); + return static_cast< NET::WindowType >( -2 ); // undefined +} /*! Updates the current client area according to the current clients. --- workspace.h.sav Fri Oct 18 13:26:35 2002 +++ workspace.h Mon Oct 21 15:38:10 2002 @@ -17,6 +17,7 @@ Copyright (C) 1999, 2000 Matthias Ettric #include "KWinInterface.h" #include #include +#include #include @@ -54,7 +55,6 @@ public: typedef QValueList SystemTrayWindowList; -struct SessionInfoPrivate; struct SessionInfo { QCString sessionId; @@ -74,9 +74,7 @@ struct SessionInfo bool staysOnTop; bool skipTaskbar; bool skipPager; - - private: - SessionInfoPrivate* d; + NET::WindowType windowType; }; @@ -415,6 +413,9 @@ private: void loadFakeSessionInfo(); void storeFakeSessionInfo( Client* c ); void writeFakeSessionInfo(); + static const char* windowTypeToTxt( NET::WindowType type ); + static NET::WindowType txtToWindowType( const char* txt ); + static bool sessionInfoWindowTypeMatch( Client* c, SessionInfo* info ); Client* active_client; Client* last_active_client; --------------Boundary-00=_B0CCERIVPUK5O19OWCFM Content-Type: text/x-diff; charset="iso-8859-1"; name="kmainwindow.cpp.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="kmainwindow.cpp.patch" --- kmainwindow.cpp.sav Tue Oct 8 18:25:10 2002 +++ kmainwindow.cpp Mon Oct 21 18:32:43 2002 @@ -29,6 +29,7 @@ #include #include #include +#include #include #include @@ -170,21 +171,35 @@ void KMainWindow::initKMainWindow(const if ( !ksm ) ksm = ksmd.setObject(new KMWSessionManaged()); - if ( !name ) { - // set a unique object name. Required by session management. - QCString s; - int unusedNumber= 0; - KMainWindow *existingWin; - do { - s.setNum( ++unusedNumber ); - s= kapp->instanceName() + "-mainwindow#" + s; - for ( existingWin= memberList->first(); existingWin!=0; - existingWin= memberList->next() ) - if ( existingWin->name() == s ) - break; - } while ( existingWin!=0 ); - setName( s ); + QCString objname; + if ( !name ) + objname = kapp->instanceName() + "-mainwindow"; + else + objname = name; + // set a unique object name. Required by session management. + // Qt should IMHO check the uniqueness of toplevel widget's name() + QCString s = objname; + int unusedNumber= 1; + for(;;) { + QWidgetList* list = kapp->topLevelWidgets(); + QWidgetListIt it( *list ); + bool found = false; + for( QWidget* w = it.current(); + w != NULL; + ++it, w = it.current()) + if( w != this && w->name() == s ) + { + found = true; + break; + } + delete list; + if( !found ) + break; + s.setNum( ++unusedNumber ); + s = objname + '#' + s; // try appending numbers } + setName( s ); + memberList->append( this ); d = new KMainWindowPrivate; --------------Boundary-00=_B0CCERIVPUK5O19OWCFM--