D13808: Fix KMainWindow saving incorrect widget settings

cfeck added a comment. Ah, unit tests are still open. Thanks for the clarification. REPOSITORY R263 KXmlGui REVISION DETAIL https://phabricator.kde.org/D13808 To: maxrd2, #kde_applications, dfaure, elvisangelaccio, broulik, cfeck Cc: anthonyfieroni, marten, asturmlechner, wbauer, aacid,

D13808: Fix KMainWindow saving incorrect widget settings

dfaure added a comment. I don't understand your question. By "this", do you mean the KMainWindow fix that was posted in a previous iteration here and that was pushed? That one is very much needed, yes, to make setAutoSaveSettings work. The change that you link to, is about porting KMail t

D13808: Fix KMainWindow saving incorrect widget settings

cfeck added a comment. Is this still needed after https://phabricator.kde.org/D14454? REPOSITORY R263 KXmlGui REVISION DETAIL https://phabricator.kde.org/D13808 To: maxrd2, #kde_applications, dfaure, elvisangelaccio, broulik, cfeck Cc: anthonyfieroni, marten, asturmlechner, wbauer, aacid

D13808: Fix KMainWindow saving incorrect widget settings

dfaure added a comment. https://phabricator.kde.org/D14454 REPOSITORY R263 KXmlGui REVISION DETAIL https://phabricator.kde.org/D13808 To: maxrd2, #kde_applications, dfaure, elvisangelaccio, broulik, cfeck Cc: anthonyfieroni, marten, asturmlechner, wbauer, aacid, ngraham, kde-frameworks-

D13808: Fix KMainWindow saving incorrect widget settings

dfaure added a comment. OK I see, this is because KMail doesn't enable autosaving, but instead saves in the KMMainWin destructor, after the window (and all children) have been hidden. I'll try whether porting KMail to setAutoSaveSettings solves it. REPOSITORY R263 KXmlGui REVISION DETAI

D13808: Fix KMainWindow saving incorrect widget settings

wbauer added a comment. In D13808#299791 , @ngraham wrote: > In the Bugzilla ticket, it was mentioned that KMail (and only KMail) still suffers from the issue even with this patch, so it might be a KMail-specific issue. For reference: h

D13808: Fix KMainWindow saving incorrect widget settings

ngraham added a comment. In the Bugzilla ticket, it was mentioned that KMail (and only KMail) still suffers from the issue even with this patch, so it might be a KMail-specific issue. REPOSITORY R263 KXmlGui REVISION DETAIL https://phabricator.kde.org/D13808 To: maxrd2, #kde_applicati

D13808: Fix KMainWindow saving incorrect widget settings

dfaure added a comment. Hmm, for me the bug is still there. Or maybe it means it's a different bug, but I kind of doubt it. Steps: 1. I start kmail (with a cleaned up ~/.config/kmail2rc to make sure the StatusBar and the Toolbar are shown). 2. I close it (Alt+F4

D13808: Fix KMainWindow saving incorrect widget settings

aacid added a comment. In D13808#293759 , @maxrd2 wrote: > Events from the window system seem to be handled after normal events. > http://code.qt.io/cgit/qt/qtbase.git/tree/src/platformsupport/eventdispatchers/qeventdispatcher_glib.cpp#n70 >

D13808: Fix KMainWindow saving incorrect widget settings

maxrd2 added a comment. Events from the window system seem to be handled after normal events. http://code.qt.io/cgit/qt/qtbase.git/tree/src/platformsupport/eventdispatchers/qeventdispatcher_glib.cpp#n70 Is there a way to send the event so qt processes it as window system event? Have tried

D13808: Fix KMainWindow saving incorrect widget settings

maxrd2 added a comment. @dfaure great.. thanks INLINE COMMENTS > anthonyfieroni wrote in kmainwindow_unittest.cpp:267 > mw->setAttribute(Qt::WA_DeleteOnClose); > Then post event close or deleteLater should work KMainWindowPrivate::init() which is called from constructor already does setAttr

D13808: Fix KMainWindow saving incorrect widget settings

maxrd2 updated this revision to Diff 37950. maxrd2 added a comment. Rebased to master REPOSITORY R263 KXmlGui CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13808?vs=37211&id=37950 BRANCH fix-window-state-save REVISION DETAIL https://phabricator.kde.org/D13808 AFFECTED FILE

D13808: Fix KMainWindow saving incorrect widget settings

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. I just pushed the fix https://commits.kde.org/kxmlgui/d35a88289513c0420863b80aa6c1cb7d2c6e978f Please continue working on the unittest here ;) REPOSITORY R263 KXmlGui REVISIO

D13808: Fix KMainWindow saving incorrect widget settings

anthonyfieroni added inline comments. INLINE COMMENTS > maxrd2 wrote in kmainwindow_unittest.cpp:267 > Retried with all of these... none of them causes the failure, only closing > the window manually cause it > > QApplication::postEvent(mw, new QDeferredDeleteEvent); // qpa/window > manager

D13808: Fix KMainWindow saving incorrect widget settings

ngraham added a comment. Yeah, I say let's remove the test for now so we can commit this, and then you can add the test back in another patch please (in working form, of course!). :) REPOSITORY R263 KXmlGui BRANCH fix-window-state-save REVISION DETAIL https://phabricator.kde.org/D1380

D13808: Fix KMainWindow saving incorrect widget settings

maxrd2 added a comment. @ngraham Don't think I can. I have no commit rights. Should i revert/remove test part, as currently it doesn't test anything i believe.. it always passes. REPOSITORY R263 KXmlGui BRANCH fix-window-state-save REVISION DETAIL https://phabricator.kde.org/D13808

D13808: Fix KMainWindow saving incorrect widget settings

ngraham added a comment. Shall we commit this? maxrd2 , can you do it, or does one of us need to commit it for you? REPOSITORY R263 KXmlGui BRANCH fix-window-state-save REVISION DETAIL https://phabricator.kde.org/D13808 To: maxrd2, #kde_applications, dfaure, elvisangelaccio, broulik,

D13808: Fix KMainWindow saving incorrect widget settings

dfaure added a comment. Hmm, good point, I thought this was in already :( REPOSITORY R263 KXmlGui BRANCH fix-window-state-save REVISION DETAIL https://phabricator.kde.org/D13808 To: maxrd2, #kde_applications, dfaure, elvisangelaccio, broulik, cfeck Cc: arojas, marten, asturmlechner, w

D13808: Fix KMainWindow saving incorrect widget settings

arojas added a comment. This has been approved for two weeks and fixes a major issue... could the fix be pushed and then keep working on the auto test later? It already missed 5.48 REPOSITORY R263 KXmlGui BRANCH fix-window-state-save REVISION DETAIL https://phabricator.kde.org/D13808

D13808: Fix KMainWindow saving incorrect widget settings

maxrd2 added inline comments. INLINE COMMENTS > broulik wrote in kmainwindow_unittest.cpp:267 > `mw->deleteLater()`? Retried with all of these... none of them causes the failure, only closing the window manually cause it QApplication::postEvent(mw, new QDeferredDeleteEvent); // qpa/window ma

D13808: Fix KMainWindow saving incorrect widget settings

maxrd2 updated this revision to Diff 37211. maxrd2 added a comment. Clened up test code. Replaced isVisible() with isHidden() test to match KMainWindow's condition. REPOSITORY R263 KXmlGui CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13808?vs=37131&id=37211 BRANCH fix-window

D13808: Fix KMainWindow saving incorrect widget settings

maxrd2 added inline comments. INLINE COMMENTS > broulik wrote in kmainwindow_unittest.cpp:267 > `mw->deleteLater()`? tried it too... will retry all of them again with isHidden() > broulik wrote in kmainwindow_unittest.cpp:278 > `QCOMPARE(mw->m-dock->isVisible(), true)` mmm.. sorry i was suppos

D13808: Fix KMainWindow saving incorrect widget settings

broulik added inline comments. INLINE COMMENTS > dfaure wrote in kmainwindow_unittest.cpp:278 > Or rather `QVERIFY(mw->m_dock->isVisible())`, that's what QVERIFY is for ;-) :D Right, I got distracted by the `==` REPOSITORY R263 KXmlGui BRANCH fix-window-state-save REVISION DETAIL https:

D13808: Fix KMainWindow saving incorrect widget settings

dfaure added inline comments. INLINE COMMENTS > broulik wrote in kmainwindow_unittest.cpp:278 > `QCOMPARE(mw->m-dock->isVisible(), true)` Or rather `QVERIFY(mw->m_dock->isVisible())`, that's what QVERIFY is for ;-) REPOSITORY R263 KXmlGui BRANCH fix-window-state-save REVISION DETAIL htt

D13808: Fix KMainWindow saving incorrect widget settings

broulik added inline comments. INLINE COMMENTS > kmainwindow_unittest.cpp:265 > +NativeMainWindow *mw = new NativeMainWindow(); > +connect(mw, &QObject::destroyed, [&](QObject *){ el.exit(); }); // > quit event loop when window's gone > + connect(mw, &QObject::destroyed, &el, &Q

D13808: Fix KMainWindow saving incorrect widget settings

wbauer added a comment. In D13808#286965 , @maxrd2 wrote: > Relative Qt Bug is here: https://bugreports.qt.io/browse/QTBUG-69277 > Seems it's not their bug. Hm. Interestingly, Qt Creator has similar problems with Qt 5.11.1 that are al

D13808: Fix KMainWindow saving incorrect widget settings

maxrd2 added a comment. Relative Qt Bug is here: https://bugreports.qt.io/browse/QTBUG-69277 Seems it's not their bug. And Qt guy commented there with: > ... You should, however, not save the window layout anymore then, because after closeEvent() any other widgets or child windows could

D13808: Fix KMainWindow saving incorrect widget settings

maxrd2 added a comment. Thank you! QEventLoop and creating window with new operator did it. Now the only problem is that no matter how i close the window from code it's not reproducing behavior. Only closing the window manually through window manager works. Have tried: QApplication::post

D13808: Fix KMainWindow saving incorrect widget settings

maxrd2 updated this revision to Diff 37131. maxrd2 added a comment. Added event loop to test and replicated wanted KMainWindow behavior. REPOSITORY R263 KXmlGui CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13808?vs=36973&id=37131 BRANCH fix-window-state-save REVISION DETAIL

D13808: Fix KMainWindow saving incorrect widget settings

maxrd2 added a comment. Have tried doing QApplication::processEvents() and QApplication::sendPostedEvents(), between main window instances... didnt change Have removed mw.close() and clicked close buttons manually... didnt change Then have started all three in separate process using QProce

D13808: Fix KMainWindow saving incorrect widget settings

aacid added a comment. In D13808#286480 , @maxrd2 wrote: > QTEST_MAIN already provides QApplication and event loop It does, but not between your NativeMainWindow mw; mw.close(); and as far as i understood the problem was wi

D13808: Fix KMainWindow saving incorrect widget settings

maxrd2 added a comment. QTEST_MAIN already provides Application and event loop REPOSITORY R263 KXmlGui BRANCH fix-window-state-save REVISION DETAIL https://phabricator.kde.org/D13808 To: maxrd2, #kde_applications, dfaure, elvisangelaccio, broulik, cfeck Cc: wbauer, aacid, ngraham, kde

D13808: Fix KMainWindow saving incorrect widget settings

aacid added a comment. Maybe you need an event loop in your tests? REPOSITORY R263 KXmlGui BRANCH fix-window-state-save REVISION DETAIL https://phabricator.kde.org/D13808 To: maxrd2, #kde_applications, dfaure, elvisangelaccio, broulik, cfeck Cc: wbauer, aacid, ngraham, kde-frameworks-

D13808: Fix KMainWindow saving incorrect widget settings

maxrd2 added a comment. Unit test is there... and it passes... always... with or without the kmainwindow.cpp patch :-/ Is there some simpler way to initialize/kill QApplication for each mainwindow creation/deletion, other than adding new test that uses QTEST_APPLESS_MAIN instead of QTEST_M

D13808: Fix KMainWindow saving incorrect widget settings

maxrd2 updated this revision to Diff 36973. maxrd2 added a comment. Added unit test REPOSITORY R263 KXmlGui CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13808?vs=36938&id=36973 BRANCH fix-window-state-save REVISION DETAIL https://phabricator.kde.org/D13808 AFFECTED FILES

D13808: Fix KMainWindow saving incorrect widget settings

maxrd2 added a comment. The change in qt affects when the window close event is handled in queue, it didn't change when it is fired. With qt 5.11.1 child widgets get destroyed a bit sooner, but it's still gets triggered by same close event - i think that stayed like it always was. Also KMa

D13808: Fix KMainWindow saving incorrect widget settings

wbauer added a comment. I just like to note that VirtualBox has the same problem with Qt 5.11.1, that the toolbar and the status bar disappear on next start if you close the window: http://bugzilla.opensuse.org/show_bug.cgi?id=1099589 So it does not only affect applications using KMainWind

D13808: Fix KMainWindow saving incorrect widget settings

aacid added a comment. Can we get an auto test? REPOSITORY R263 KXmlGui BRANCH fix-window-state-save REVISION DETAIL https://phabricator.kde.org/D13808 To: maxrd2, #kde_applications, dfaure, elvisangelaccio, broulik, cfeck Cc: aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D13808: Fix KMainWindow saving incorrect widget settings

dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Hard to tell if the qt behavior change is a bug or a feature, but the fix makes sense to me. (BTW I've been experiencing this regression in zanshin) REPOSITORY R263 KXmlGui BRANCH

D13808: Fix KMainWindow saving incorrect widget settings

ngraham added reviewers: KDE Applications, dfaure, elvisangelaccio, broulik, cfeck. ngraham added a comment. The bug report indicates that this patch is for adapting to a change in Qt 5.11. Is this patch fully backwards-compatible with older Qt versions? If so, we'll need to guard this behin

D13808: Fix KMainWindow saving incorrect widget settings

maxrd2 added a comment. Relative bug is here https://bugs.kde.org/show_bug.cgi?id=395988 REPOSITORY R263 KXmlGui REVISION DETAIL https://phabricator.kde.org/D13808 To: maxrd2 Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13808: Fix KMainWindow saving incorrect widget settings

maxrd2 updated this revision to Diff 36938. maxrd2 added a comment. Cleaned up indentation. REPOSITORY R263 KXmlGui CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13808?vs=36937&id=36938 BRANCH fix-window-state-save REVISION DETAIL https://phabricator.kde.org/D13808 AFFECTE

D13808: Fix KMainWindow saving incorrect widget settings

maxrd2 created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. maxrd2 requested review of this revision. REVISION SUMMARY BUG: 395988 In certain cases KMainWindow::saveMainWindowSettings() could have been cal