MAINT: Make PdfFileMerger.addBookmark() behave life PdfFileWriters' (#339)

People stumbled over this inconsistency:

* #40 
* https://stackoverflow.com/a/42991101/562769

This was also tested with:

https://stackoverflow.com/questions/42941742/pypdf2-nested-bookmarks-with-same-name-not-working/42991101#comment73249244_42991101

Author: khalida

PyPDF2/merger.py

@@ -498,36 +498,70 @@ def findBookmark(self, bookmark, root=None):
 
         return None
 
-    def addBookmark(self, title, pagenum, parent=None):
+
+    def addBookmark(self, title, pagenum, parent=None, color=None, bold=False, italic=False, fit='/Fit', *args):
         """
         Add a bookmark to this PDF file.
 
         :param str title: Title to use for this bookmark.
         :param int pagenum: Page number this bookmark will point to.
         :param parent: A reference to a parent bookmark to create nested
             bookmarks.
+        :param tuple color: Color of the bookmark as a red, green, blue tuple
+            from 0.0 to 1.0
+        :param bool bold: Bookmark is bold
+        :param bool italic: Bookmark is italic
+        :param str fit: The fit of the destination page. See
+            :meth:`addLink()<addLin>` for details.
         """
-        if parent is None:
-            iloc = [len(self.bookmarks)-1]
-        elif isinstance(parent, list):
-            iloc = parent
+        if len(self.output.getObject(self.output._pages)['/Kids']) > 0:
+            pageRef = self.output.getObject(self.output._pages)['/Kids'][pagenum]
         else:
-            iloc = self.findBookmark(parent)
+            pageRef = self.output.getObject(self.output._pages)
 
-        dest = Bookmark(TextStringObject(title), NumberObject(pagenum), NameObject('/FitH'), NumberObject(826))
+        action = DictionaryObject()
+        zoomArgs = []
+        for a in args:
+            if a is not None:
+                zoomArgs.append(NumberObject(a))
+            else:
+                zoomArgs.append(NullObject())
+        dest = Destination(NameObject("/"+title + " bookmark"), pageRef, NameObject(fit), *zoomArgs)
+        destArray = dest.getDestArray()
+        action.update({
+            NameObject('/D') : destArray,
+            NameObject('/S') : NameObject('/GoTo')
+        })
+        actionRef = self.output._addObject(action)
+
+        outlineRef = self.output.getOutlineRoot()
 
         if parent is None:
-            self.bookmarks.append(dest)
-        else:
-            bmparent = self.bookmarks
-            for i in iloc[:-1]:
-                bmparent = bmparent[i]
-            npos = iloc[-1]+1
-            if npos < len(bmparent) and isinstance(bmparent[npos], list):
-                bmparent[npos].append(dest)
-            else:
-                bmparent.insert(npos, [dest])
-        return dest
+            parent = outlineRef
+
+        bookmark = TreeObject()
+
+        bookmark.update({
+            NameObject('/A'): actionRef,
+            NameObject('/Title'): createStringObject(title),
+        })
+
+        if color is not None:
+            bookmark.update({NameObject('/C'): ArrayObject([FloatObject(c) for c in color])})
+
+        format = 0
+        if italic:
+            format += 1
+        if bold:
+            format += 2
+        if format:
+            bookmark.update({NameObject('/F'): NumberObject(format)})
+
+        bookmarkRef = self.output._addObject(bookmark)
+        parent = parent.getObject()
+        parent.addChild(bookmarkRef, self.output)
+
+        return bookmarkRef
 
     def addNamedDestination(self, title, pagenum):
         """

Tests/test_merger.py

@@ -51,6 +51,7 @@ def test_merge():
     # Check if bookmarks are correct
     pdfr = PyPDF2.PdfFileReader(tmp_path)
     assert [el.title for el in pdfr.getOutlines() if isinstance(el, Destination)] == [
+        "A bookmark",
         "Foo",
         "Bar",
         "Baz",
@@ -61,7 +62,6 @@ def test_merge():
         "Bar",
         "Baz",
         "True",
-        "A bookmark",
     ]
 
     # Clean up