D11051: Remembering side navigation panel state

ngraham abandoned this revision. ngraham added a comment. This is on GitLab now: https://invent.kde.org/kde/okular/merge_requests/22 REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D11051 To: ngraham, #okular, aacid, dileepsankhla Cc: okular-devel, tobiasdeiminger,

D11051: Remembering side navigation panel state

ngraham commandeered this revision. ngraham added a reviewer: dileepsankhla. Herald added a subscriber: okular-devel. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D11051 To: ngraham, #okular, aacid, dileepsankhla Cc: okular-devel, tobiasdeiminger, aacid, sander,

D11051: Remembering side navigation panel state

dileepsankhla added a comment. Currently I am working as a GSoC student and in discussion with my mentor, I have decided to pause the patch report during the GSoC period. BBL REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D11051 To: dileepsankhla, #okular, aacid Cc:

D11051: Remembering side navigation panel state

aacid requested changes to this revision. This revision now requires changes to proceed. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D11051 To: dileepsankhla, #okular, aacid Cc: tobiasdeiminger, aacid, sander, #okular, ngraham

D11051: Remembering side navigation panel state

aacid added a comment. In D11051#239497 , @dileepsankhla wrote: > The state can be saved whenever a sidebar item is clicked in Sidebar::itemClicked but again as discussed earlier, it will be asymmetrical as to save in sidebar.cpp. Should I

D11051: Remembering side navigation panel state

dileepsankhla added a comment. The state can be saved whenever a sidebar item is clicked in Sidebar::itemClicked but again as discussed earlier, it will be asymmetrical as to save in sidebar.cpp. Should I implement a signal slot mechanism in part.cpp to achieve so or should I find another

D11051: Remembering side navigation panel state

aacid added a comment. In D11051#239040 , @tobiasdeiminger wrote: > > One thing that may fix it is changing > > > > m_sidebar = new Sidebar( parentWidget ); > > > > to > > > > m_sidebar = new Sidebar(); > > > > in

D11051: Remembering side navigation panel state

aacid added a comment. In D11051#238586 , @dileepsankhla wrote: > I tried but it is still giving me the segfault. The interesting thing, in my opinion, is whenever you open Okular and close it down without changing the sidebar state, it

D11051: Remembering side navigation panel state

tobiasdeiminger added a comment. > One thing that may fix it is changing > > m_sidebar = new Sidebar( parentWidget ); > > to > > m_sidebar = new Sidebar(); > > in part.cpp, this micht change the part/child relationships and get yourself of this problem. can you give it

D11051: Remembering side navigation panel state

dileepsankhla added a comment. I tried but it is still giving me the segfault. The interesting thing, in my opinion, is whenever you open Okular and close it down without changing the sidebar state, it doesn't give a segfault whereas changing the sidebar state and closing it gives the

D11051: Remembering side navigation panel state

aacid added a comment. So https://paste.kde.org/pwvknplv2 shows the problem Shell is deleting m_tabWidget which is deleting the sidebars before the parts are deleted. Do you understand that from reading that piece of text? One thing that may fix it is changing m_sidebar

D11051: Remembering side navigation panel state

dileepsankhla added a comment. The verbose output of valgrind is around 190,000 lines with the line numbers. Here is the output: https://dileepsankhla.com/valgrind.txt REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D11051 To: dileepsankhla, #okular, aacid Cc:

D11051: Remembering side navigation panel state

aacid added a comment. This is the important part https://paste.kde.org/pldoxclam It tells you that you're trying to use Sidebar while it was already deleted. Problem is you did not compile with debug enabled and the log is not as useful as it could. Please add -DCMAKE_BUILD_TYPE=Debug

D11051: Remembering side navigation panel state

dileepsankhla added a comment. https://paste.kde.org/pepnfuzxq REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D11051 To: dileepsankhla, #okular, aacid Cc: aacid, sander, #okular, michaelweghorn, ngraham

D11051: Remembering side navigation panel state

aacid added a comment. In D11051#237655 , @dileepsankhla wrote: > Following is the log of running valgrind on okular: > > https://paste.kde.org/pmyjdscmw/ohzvag This paste is password

D11051: Remembering side navigation panel state

dileepsankhla added a comment. Following is the log of running valgrind on okular: https://paste.kde.org/pmyjdscmw/ohzvag I have no luck in understanding the verbose. May you please help me by checking it out? REPOSITORY R223 Okular

D11051: Remembering side navigation panel state

aacid added a comment. In D11051#235502 , @dileepsankhla wrote: > Actually I am getting the segmentation fault whenever I close Okular with a changed state of the side navigation panel and the code causing it is

D11051: Remembering side navigation panel state

dileepsankhla updated this revision to Diff 30742. dileepsankhla added a comment. Updating: Remembring side navigation panel state Changing the state to the one being discussed here in comments. REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE

D11051: Remembering side navigation panel state

dileepsankhla added a comment. Actually I am getting the segmentation fault whenever I close Okular with a changed state of the side navigation panel and the code causing it is `Okular::Settings::setHideSideContainer( m_sidebar->isCollapsed() )` inside the Part destructor. `m_sidebar` is

D11051: Remembering side navigation panel state

aacid requested changes to this revision. This revision now requires changes to proceed. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D11051 To: dileepsankhla, #okular, aacid Cc: aacid, sander, #okular, michaelweghorn, ngraham

D11051: Remembering side navigation panel state

aacid added a comment. In D11051#222803 , @dileepsankhla wrote: > In D11051#05 , @aacid wrote: > > > Also i'm not convinced it's a good idea to save the settings in sidebar.cpp but read them in

D11051: Remembering side navigation panel state

dileepsankhla marked an inline comment as done. dileepsankhla added a comment. In D11051#05 , @aacid wrote: > Also i'm not convinced it's a good idea to save the settings in sidebar.cpp but read them in part.cpp, seems a bit asymmetrical,

D11051: Remembering side navigation panel state

aacid added a comment. Do you think you can give it a try at adding an auto test for this? Also i'm not convinced it's a good idea to save the settings in sidebar.cpp but read them in part.cpp, seems a bit asymmetrical, can you explain your reasoning for it? REPOSITORY R223 Okular

D11051: Remembering side navigation panel state

sander added a comment. I tried the patch and it does what it claims to do. I have no further objections. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D11051 To: dileepsankhla, #okular Cc: sander, #okular, michaelweghorn, ngraham, aacid

D11051: Remembering side navigation panel state

dileepsankhla updated this revision to Diff 28694. dileepsankhla added a comment. Updating: Remembering side navigation panel state Removed the extra whitespace. REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11051?vs=28693=28694 BRANCH master

D11051: Remembering side navigation panel state

sander added inline comments. INLINE COMMENTS > sidebar.cpp:717 > { > + > if ( !item ) Please avoid such white-space changes. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D11051 To: dileepsankhla, #okular Cc: sander, #okular, michaelweghorn, ngraham, aacid

D11051: Remembering side navigation panel state

dileepsankhla added a reviewer: Okular. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D11051 To: dileepsankhla, #okular Cc: #okular, michaelweghorn, ngraham, aacid

D11051: Remembering side navigation panel state

dileepsankhla created this revision. Restricted Application added a subscriber: Okular. Restricted Application added a project: Okular. dileepsankhla requested review of this revision. REVISION SUMMARY Every time Okular starts, one of the items of the side navigation panel is open. There is no