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

My one concern about this is that I don't understand why the unit test was passing before (if it ever was). "git blame" shows that this logic has been this way since the code was first added several years ago.

- Robert Mathias

On March 13th, 2012, 6:59 p.m., Robert Mathias Marmorstein wrote:

Review request for KOffice. By Robert Mathias Marmorstein. Updated March 13, 2012, 6:59 p.m. Description The TestPACopyPastePage unit test was failing with a segmentation fault (signal 11). Debugging it in gdb showed that the cause was a null pointer for the "page" variable in a loop through the master pages. Tracing this back, I discovered some odd code that seemed to be dynamic_casting a Page* to a Page*. I'm not sure that behaviour is even defined in the C++ standard, but I'm pretty sure what we really want to do is cast to a MasterPage instead (and reverse the logic of the if statement). So that's what this patch does. Testing Code still compiles and runs and master pages work. The TestPACopyPastePage unit test now passes instead of failing and all other unit tests still pass (except the two that were failing already -- both related to layout problems). Diffs libs/kopageapp/KoPAOdfPageSaveHelper.cpp (1a8f965) View Diff

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

Ship it!

The change looks correct. The change made by me some time ago (2nd christmas day 2011) where there used to be a KoPAPageBase seems to me a likely cause of this bug, apologies for not finding the unit test regression, and thanks for the fix!

- Thomas

On March 13th, 2012, 6:59 p.m., Robert Mathias Marmorstein wrote:

Review request for KOffice. By Robert Mathias Marmorstein. Updated March 13, 2012, 6:59 p.m. Description The TestPACopyPastePage unit test was failing with a segmentation fault (signal 11). Debugging it in gdb showed that the cause was a null pointer for the "page" variable in a loop through the master pages. Tracing this back, I discovered some odd code that seemed to be dynamic_casting a Page* to a Page*. I'm not sure that behaviour is even defined in the C++ standard, but I'm pretty sure what we really want to do is cast to a MasterPage instead (and reverse the logic of the if statement). So that's what this patch does. Testing Code still compiles and runs and master pages work. The TestPACopyPastePage unit test now passes instead of failing and all other unit tests still pass (except the two that were failing already -- both related to layout problems). Diffs libs/kopageapp/KoPAOdfPageSaveHelper.cpp (1a8f965) View Diff