> On May 19, 2013, 4:36 a.m., Thorsten Zachmann wrote:
> > plugins/textshape/dialogs/DockerStylesComboModel.cpp, lines 156-159
> > <http://git.reviewboard.kde.org/r/110506/diff/1/?file=144929#file144929line156>
> >
> >     I'm not 100% sure but I think the test for the stylesType is not needed 
> > as a default style should never be added, but it surely does not harm 
> > either.

exactly the same thought i had when looking through the code yesterday. I don't 
feel overly confident in approving code so will wait for pierre


- C.


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


On May 18, 2013, 3:39 p.m., Elvis Stansvik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110506/
> -----------------------------------------------------------
> 
> (Updated May 18, 2013, 3:39 p.m.)
> 
> 
> Review request for Calligra and Pierre Stirnweiss.
> 
> 
> Description
> -------
> 
> During the loading of the attached document (from bug #319048 ), 
> DockerStylesComboModel::styleApplied is called with the default paragraph 
> style as argument.
> 
> The function assumes that the source model can provide an index for the 
> supplied style (see the calls to indexForCharacterStyle). But the source 
> model explicitly avoids adding items for the default styles, so these calls 
> will return an invalid index, which results in a -1 being added as a row to 
> m_usedStyles. In the next call to styleApplied, this will result in an 
> invalid internalId being used to try to get the corresponding style from the 
> style manager. The invalid style pointer is then used, resulting in the crash 
> described in the bug.
> 
> The attached patch turns DockerStylesComboModel::styleApplied into a no-op if 
> the supplied style is either the default character style or the default 
> paragraph styles, since there will never be any items for these in the source 
> model.
> 
> I'm not sure this is the correct fix, so would be great if e.g. Pierre could 
> have a look.
> 
> 
> This addresses bug 319048.
>     http://bugs.kde.org/show_bug.cgi?id=319048
> 
> 
> Diffs
> -----
> 
>   plugins/textshape/dialogs/DockerStylesComboModel.cpp 40ae007 
> 
> Diff: http://git.reviewboard.kde.org/r/110506/diff/
> 
> 
> Testing
> -------
> 
> Tried to load the attached document before/after the fix. And the problem 
> seems solved.
> 
> 
> File Attachments
> ----------------
> 
> The ODT provoking the crash
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/05/18/space.odt
> 
> 
> Thanks,
> 
> Elvis Stansvik
> 
>

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

Reply via email to