Fixes for InsertDocumentPageAt

[dprandle]

• [ x] Accept to license the library and tools code under the terms
  of the LGPL 2.0 or later
• [ x] Accept to license the library and tools code under the terms
  of the MPL 2.0
• [ x] Accept to license the test code under the terms
  of the MIT-0
• [ x] Read contributions guidelines
• [ x] Checked coding style
• [ x] The commits sequence is clean without work in progress/bugged revisions and merge commits. In doubt, squash the commits and/or rebase master

The fix in the commit 202f6af ("Add free objects from other doc not self") is necessary for the InsertDocumentPageAt function to work at all.

[ceztko]

As stated elsewhere in general I don't like the append code and I want to completely rewrite it at some time. Anyway, about your commit ef750ac, I have a deja-vu where I fixed an identical issue and in fact there was some previously duplicated code few lines above, see this commit (that's why duplicated code is pretty much always evil). I don't like the approach to remove functionalities instead of just fixing a bug: please review my commit and if it fixes the bug as well for you use the same identical approach for that commit of yours.

[dprandle]

see this commit

I don't like the approach to remove functionalities instead of just fixing a bug: please review my commit and if it fixes the bug as well for you use the same identical approach for that commit of yours.

I took this approach at first - it will indeed fix the crash by creating the outline in the destination document.

However, this function (InsertDocumentPageAt) is not for appending one document to another (and indeed our code doesn't use it that way). We use this function to cherry pick pages from multiple source documents and conglomerate them in to a single final document.

Since this function appends a single page from a source document to the destination document, it really makes no sense to append the source document's outline. And each page you insert from the source document would append the source document's outline again. Image a source document of 100 pages with an outline with 15 items - if you insert pages 4 and 36 from that source document you pull in the entire outline from the source document for each page you insert.

With all of that being said, it seems removing the code to append the source document's outline to the destination document in InsertDocumentPageAt is most appropriate.

[ceztko]

With all of that being said, it seems removing the code to append the source document's outline to the destination document in InsertDocumentPageAt is most appropriate.

I see, with this explanation the change makes a lot more sense. Please, give me some more time (I'm very busy these days) to review the other changes but at least consider this one sorted.

[ceztko]

68d2a5d doesn't belong here, so please drop it. it shouldn't appear in the history, so create a separate PR for it (I will review it quickly), and do git reset --hard HEAD^ here so it will remove the last commit and force push.

Clarified the meaning of the change

[dprandle]

Yes - sorry I didn't mean to commit that to this branch actually

[ceztko]

Every looked fine, thank you. Sorry for the delay in reviewing: the changes were easy but it was a portion of code I am not very confident so I waited the first free time moment to have a look at them.