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

[Stephan Binner]

> On April 10, 2015, 11:43 a.m., Stephan Binner wrote:
> > shell/shell.cpp, line 72
> > <https://git.reviewboard.kde.org/r/122570/diff/6/?file=352461#file352461line72>
> >
> >     Was this change intentional? It breaks proper detection of EmbedMode. :-(
> 
> Jonathan Doman wrote:
>     Yes, it was intentional. See my previous comment: "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)"
>     
>     However, I'm no expert on that, I was just following documentation. It can be reverted if needed.

No, don't revert but please adapt
    core/script/kjs_document.cpp:                           && widget->parentWidget()->objectName() == QLatin1String( "okular::Shell" ) );
    part.cpp:         && ( parent->objectName() == QLatin1String( "okular::Shell" )
places to match string start.


- Stephan


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


On March 16, 2015, 10:56 p.m., Jonathan Doman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122570/
> -----------------------------------------------------------
> 
> (Updated March 16, 2015, 10:56 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> 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
> -----
> 
>   part.h 594eb44113ae130a6fefbf2800af32886aa3cbef 
>   part.cpp 36438af1cd1036ee954f80b5359a0cab2c019036 
>   shell/main.cpp 16289608f0acf299db04258d842bbb87add62c0b 
>   shell/shell.h 224acfe023ef8e9cc58b52ddf32068af8937896a 
>   shell/shell.cpp f7675fdc8203e90210b8ba82620b19ae69ee43e1 
>   tests/mainshelltest.cpp c5d7289d668f8a1ea0250deb068a43c19490edff 
> 
> 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/20150410/7321cf30/attachment.html>