BUG: Fix missing page for bookmark (#1016)

Remove code duplication Closes #968 (introduced with #339)
py-pdf · Jun 23, 2022 · a412e26 · a412e26
1 parent 7d820f0
commit a412e26
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 44 deletions.
diff --git a/PyPDF2/_merger.py b/PyPDF2/_merger.py
@@ -46,7 +46,6 @@
     NumberObject,
     TextStringObject,
     TreeObject,
-    _create_bookmark,
 )
 from .pagerange import PageRange, PageRangeSpec
 from .types import (
@@ -55,7 +54,6 @@
     LayoutType,
     OutlinesType,
     PagemodeType,
-    ZoomArgsType,
     ZoomArgType,
 )
 
@@ -658,44 +656,12 @@ def add_bookmark(
         :param str fit: The fit of the destination page. See
             :meth:`addLink()<addLink>` for details.
         """
-        if self.output is None:
+        writer = self.output
+        if writer is None:
             raise RuntimeError(ERR_CLOSED_WRITER)
-        out_pages = cast(Dict[str, Any], self.output.get_object(self.output._pages))
-        if len(out_pages["/Kids"]) > 0:
-            page_ref = out_pages["/Kids"][pagenum]
-        else:
-            page_ref = out_pages
-
-        action = DictionaryObject()
-        zoom_args: ZoomArgsType = []
-        for a in args:
-            if a is not None:
-                zoom_args.append(NumberObject(a))
-            else:
-                zoom_args.append(NullObject())
-        dest = Destination(
-            NameObject("/" + title + " bookmark"), page_ref, NameObject(fit), *zoom_args
-        )
-        dest_array = dest.dest_array
-        action.update(
-            {NameObject("/D"): dest_array, NameObject("/S"): NameObject("/GoTo")}
+        return writer.add_bookmark(
+            title, pagenum, parent, color, bold, italic, fit, *args
         )
-        action_ref = self.output._add_object(action)
-
-        outline_ref = self.output.get_outline_root()
-
-        if parent is None:
-            parent = outline_ref
-
-        bookmark = _create_bookmark(action_ref, title, color, italic, bold)
-
-        assert parent is not None, "hint for mypy"
-        bookmark_ref = self.output._add_object(bookmark)
-        parent_obj = cast(Bookmark, parent.get_object())
-        assert parent_obj is not None, "hint for mypy"
-        parent_obj.add_child(bookmark_ref, self.output)
-
-        return bookmark_ref
 
     def addNamedDestination(self, title: str, pagenum: int) -> None:  # pragma: no cover
         """

diff --git a/PyPDF2/_writer.py b/PyPDF2/_writer.py
@@ -1068,8 +1068,7 @@ def add_bookmark(
         :param str fit: The fit of the destination page. See
             :meth:`addLink()<addLink>` for details.
         """
-        pages_obj = cast(Dict[str, Any], self.get_object(self._pages))
-        page_ref = pages_obj[PA.KIDS][pagenum]
+        page_ref = NumberObject(pagenum)
         action = DictionaryObject()
         zoom_args: ZoomArgsType = []
         for a in args:

diff --git a/tests/test_merger.py b/tests/test_merger.py
@@ -45,10 +45,8 @@ def test_merge():
         merger.append(fh)
 
     bookmark = merger.add_bookmark("A bookmark", 0)
-    bm2 = merger.add_bookmark("deeper", 1, parent=bookmark, italic=True, bold=True)
-    merger.add_bookmark(
-        "Let's see", merger.pages[2], bm2, (255, 255, 0), True, True, "/FitBV", 12
-    )
+    bm2 = merger.add_bookmark("deeper", 0, parent=bookmark, italic=True, bold=True)
+    merger.add_bookmark("Let's see", 2, bm2, (255, 255, 0), True, True, "/FitBV", 12)
     merger.add_bookmark(
         "The XYZ fit", 0, bookmark, (255, 0, 15), True, True, "/XYZ", 10, 20, 3
     )