[Okular-devel] Review Request 122570: Update session restore/save to account for multiple tabs

Jonathan Doman jonathan.doman at gmail.com
Sun Feb 22 05:10:45 UTC 2015


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122570/
-----------------------------------------------------------

(Updated Feb. 21, 2015, 11:10 p.m.)


Review request for Okular.


Changes
-------

Ensure that viewport is saved by properly closing each Part (`part->closeUrl()`). When the application is quit during session tear down, the Shell destructor is not called, so the same logic is now triggered by `QApplication::aboutToQuit` signal.

Also a few cleanup changes:
1. RESTORE macro is replaced with preferred kRestoreMainWindows function. [ref](http://api.kde.org/4.x-api/kdelibs-apidocs/kdeui/html/kmainwindow_8h.html#a59a5929cb898cca8ac26a3d55397aae3)
2. Shell object name is made unique as recommended by docs [ref](http://api.kde.org/4.x-api/kdelibs-apidocs/kdeui/html/classKMainWindow.html#ab2f7f5f30e920d8490f3f83fe46149d0)
3. Remove .moc include since it isn't necessary and breaks my editing workflow (compiler-enabled code completion doesn't work when moc file is outside source tree).


Still have yet to as tests.


Bugs: 335852
    http://bugs.kde.org/show_bug.cgi?id=335852


Repository: okular


Description
-------

New Shell logic loops through each tab and saves URLs and active tab index in session config.

Viewport was previously saved in session config, but I opted to remove it because:
1. It complicates the restore logic. It would require either using QMetaObject::invoke or adding functions to ViewerInterface. Also hard to get right since opening a document isn't exactly synchronous.
2. Viewport info is already saved during a graceful shutdown.


Diffs (updated)
-----

  part.h 594eb44113ae130a6fefbf2800af32886aa3cbef 
  part.cpp 36438af1cd1036ee954f80b5359a0cab2c019036 
  shell/main.cpp 16289608f0acf299db04258d842bbb87add62c0b 
  shell/shell.h 224acfe023ef8e9cc58b52ddf32068af8937896a 
  shell/shell.cpp f7675fdc8203e90210b8ba82620b19ae69ee43e1 

Diff: https://git.reviewboard.kde.org/r/122570/diff/


Testing
-------

I was not familiar with session functionality in KDE before working on this bug, so my tests may not represent reality. I used the dbus interface to trigger a session save (org.kde.KSMServerInterface.saveCurrentSession), and then reloaded a session by running `okular --session xyz`, which I think is how KDE does it behind the scenes.

- Restore one or more documents in single window with tabs enabled.
- Restore multiple windows, tabs enabled or disabled.
- Restore session config describing multible tabs, even though tabs are disabled


Thanks,

Jonathan Doman

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20150222/895cbc69/attachment.html>


More information about the Okular-devel mailing list