BUG: Make remove_from_tree() work on outline items

[RISHIKKASULA]

What this fixes

#3878 reports that remove_from_tree() fails on outline items whose colour is [0.0, 0.0, 0.0]. The colour is not the trigger. Destination.color returns black as its default when /C is absent, so on the reporter's PDF — where no bookmark carries a colour at all — that filter matched every item in the document.

The actual problem is that reader.outline and writer.outline never hand back the live outline nodes. _build_outline_item constructs a fresh Destination from each node's fields and does not copy /Parent, because a detached copy is not in any tree. Destination subclasses TreeObject and so inherits remove_from_tree(), which checks self for the /Parent that copy can never have.

This fails on every PDF and every outline item. Destination.node does point at the real node, but that node is a plain DictionaryObject, so item.node.remove_from_tree() raises AttributeError. As far as I can tell there is currently no way to remove an outline item through the public API.

The change

remove_from_tree() acts on the node the item was built from, via a _tree_node() hook that Destination overrides to return self.node. The plumbing already existed and was documented on the class; nothing used it.

The parent is also upgraded to a TreeObject where needed. Outline parents read from a document arrive as plain DictionaryObjects unless PdfWriter.get_outline_root() has upgraded them, which it only does for the root, so the existing cast() had no effect at runtime. TreeObject adds methods and no state, so the dictionary, its identity and its indirect reference are unchanged.

Verification

Five tests added to tests/test_generic.py, next to the existing remove_from_tree tests. All five fail on main and pass with this change. They cover removing the first, middle and last item of a cloned writer, a writer that built its own outline, and a nested child. The parametrised cases also assert the result survives write() and a reload, which is what proves the mutation reached the document rather than a copy.

tests/test_generic.py goes from 111 to 116 passing with no change to the pre-existing failures. -k "outline or bookmark or tree" across tests/ is identical before and after. mypy and ruff check are clean on the changed file. Also checked against the input.pdf attached to the issue: the reporter's own loop now removes the items and the reloaded file agrees.

One thing I would like your opinion on

parent.__class__ = TreeObject upgrades in place because TreeObject(parent) copies the dictionary, which would send the mutation to the copy. get_outline_root() solves the same problem by copying and then calling _replace_object, but that needs writer access this code does not have. Happy to reshape it if you would rather it were done another way.

Closes #3878

[codecov]

Codecov Report

❌ Patch coverage is 44.44444% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.87%. Comparing base (8b6f6fd) to head (b64c81d).
⚠️ Report is 26 commits behind head on main.

Files with missing lines	Patch %	Lines
pypdf/generic/_data_structures.py	44.44%	3 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3939      +/-   ##
==========================================
- Coverage   97.88%   97.87%   -0.02%     
==========================================
  Files          57       57              
  Lines       10813    10988     +175     
  Branches     2025     2057      +32     
==========================================
+ Hits        10584    10754     +170     
- Misses        127      130       +3     
- Partials      102      104       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[stefan6419846]

Thanks for the PR. I have left some inline comments.

[RISHIKKASULA]

Thanks — that's a useful set of comments, and the third and fourth between them pointed at a
better shape than what I pushed. Reworked.

On _tree_node, and "doesn't self always live in the tree?" — No, and that is the whole
bug. For a TreeObject self is the node. But reader.outline and writer.outline do not
yield nodes: _build_outline_item constructs a fresh Destination from each node's fields, so
self there is a detached copy that never carries /Parent. You are right that TreeObject
should not know which subclass overrides what, so the _tree_node hook is gone and the special
case now lives in Destination.remove_from_tree, where the special case actually is.
TreeObject.remove_from_tree is unchanged apart from your "/Parent" suggestion.

On parent.__class__ = TreeObject — "is this really required?" — It was, but only because
remove_child reaches self._remove_node_from_tree. Outline parents read from a document are
plain DictionaryObjects — only PdfWriter.get_outline_root() upgrades anything, and only the
root — so calling remove_child on one raised
AttributeError: 'DictionaryObject' object has no attribute '_remove_node_from_tree'. Rather
than mutate the class, I have bound that one internal call explicitly, which is the only change
remove_child needs. The class mutation is gone.

Tests — moved to tests/generic/test_data_structures.py.

The isinstance(entry, list) check — gone. It was there because outline nests children in
lists, but only one test needs the nested form, so it now indexes directly.

The nested index — yes, writer.outline[1][0]. Changed.

Rechecked after the rework: the five tests fail on main and pass here, ruff check . and
mypy are clean, and the input.pdf from #3878 still works end to end.

[stefan6419846]

Please use single backticks.

[stefan6419846]

This condition and the following condition are untested. Please ensure to test them for proper coverage.