> On Dec. 28, 2012, 11:56 p.m., Albert Astals Cid wrote:
> > Why did you add an "id"? What is that "id" supposed to mean?
> 
> Jaydeep Solanki wrote:
>     because when the display name of two nodes are same, okular won't know 
> which one to expand & will end up expanding the wrong one at times. So id 
> gives a unique identity to each node, to avoid ambiguity.

Right, i see the problem.

Could you try storing QModelIndexes instead of the "id" you just have created? 
QModelIndex should work fine since it already has the parent structure, etc, so 
it should be "doable".

The "problem" i see with the current "id" thing is that are adding more memory 
usage for something that doesn't seem to be necessary.

Also you need to delete/clear m_oldTocModel and m_expandedList after the reload 
is done, otherwise we are leaking memory (in the first case) or using more 
memory than needed (in the second case)

There's also a few minor "style" issue i'll comment in a moment.


- Albert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107994/#review24157
-----------------------------------------------------------


On Dec. 28, 2012, 11:15 p.m., Jaydeep Solanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107994/
> -----------------------------------------------------------
> 
> (Updated Dec. 28, 2012, 11:15 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Description
> -------
> 
> This fix will preserve the state of the Table Of Content (toc) on document 
> reload, but if the toc is edited/changed, then it will be restored to default 
> (i.e. all nodes will be folded)
> 
> 
> This addresses bug 312138.
>     http://bugs.kde.org/show_bug.cgi?id=312138
> 
> 
> Diffs
> -----
> 
>   part.h 0c57560 
>   part.cpp 1922128 
>   ui/toc.h eeeff98 
>   ui/toc.cpp 4c84b62 
>   ui/tocmodel.h a857dc0 
>   ui/tocmodel.cpp 39add80 
> 
> Diff: http://git.reviewboard.kde.org/r/107994/diff/
> 
> 
> Testing
> -------
> 
> Checked. Works fine with me.
> 
> 
> Thanks,
> 
> Jaydeep Solanki
> 
>

_______________________________________________
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel

Reply via email to