Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions src/pptx/oxml/shapes/groupshape.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,16 +149,24 @@ def iter_shape_elms(self) -> Iterator[ShapeElement]:

@property
def max_shape_id(self) -> int:
"""Maximum int value assigned as @id in this slide.

This is generally a shape-id, but ids can be assigned to other
objects so we just check all @id values anywhere in the document
(XML id-values have document scope).

In practice, its minimum value is 1 because the spTree element itself
is always assigned id="1".
"""Maximum int value assigned as @id within this shape tree.

This is the maximum @id value on any descendant of this `<p:spTree>` (or
`<p:grpSp>`) element. Scoped strictly to the shape tree because OOXML
uses sibling element-types (notably `<p:sldLayoutIdLst>/<p:sldLayoutId>`
on a slide master) whose `@id` values live in a deliberately-segregated
namespace starting at 2147483649. Pulling those values into this max
would push the next shape id beyond `xs:int`, which PowerPoint quarantines
on open ("Repair", with the offending shape removed).

In practice the minimum return is 1 because the spTree element itself is
always assigned `id="1"`.
"""
id_str_lst = self.xpath("//@id")
# ---`.//@id` (not `//@id`) — `//` is absolute in XPath and scopes to the
# whole document, picking up sibling-element ids that share neither the
# semantic namespace nor the id range with shape ids. See the docstring
# for the failure mode that surfaced via SlideMaster.add_text_watermark.
id_str_lst = self.xpath(".//@id")
used_ids = [int(id_str) for id_str in id_str_lst if id_str.isdigit()]
return max(used_ids) if used_ids else 0

Expand Down
35 changes: 35 additions & 0 deletions tests/oxml/shapes/test_groupshape.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,41 @@ def it_calculates_its_child_extents_to_help(self, child_exts_fixture):
x, y, cx, cy = xSp._child_extents
assert (x, y, cx, cy) == expected_values

def it_returns_zero_for_max_shape_id_on_an_empty_spTree(self):
spTree = element("p:spTree")
assert spTree.max_shape_id == 0

def it_finds_the_max_id_among_shape_descendants(self):
spTree = element(
"p:spTree/(p:nvGrpSpPr/p:cNvPr{id=1,name=root}"
",p:sp/p:nvSpPr/p:cNvPr{id=2,name=a}"
",p:sp/p:nvSpPr/p:cNvPr{id=42,name=b}"
",p:sp/p:nvSpPr/p:cNvPr{id=7,name=c})"
)
assert spTree.max_shape_id == 42

def but_it_ignores_sibling_ids_outside_the_shape_tree(self):
# ---Regression for the slide-master shape-id-overflow bug surfaced by
# SlideMaster.add_text_watermark: `<p:sldLayoutIdLst>` lives as a
# sibling of `<p:cSld>` on a slide master and uses sentinel ids
# starting at 2147483649. A whole-document xpath would pull those
# values in and push the next shape id past `xs:int`, which
# PowerPoint quarantines on open ("Repair" path).
sldMaster = element(
"p:sldMaster/(p:cSld/p:spTree/(p:nvGrpSpPr/p:cNvPr{id=1,name=root}"
",p:sp/p:nvSpPr/p:cNvPr{id=2,name=title})"
",p:sldLayoutIdLst/(p:sldLayoutId{id=2147483649,r:id=rId1}"
",p:sldLayoutId{id=2147483650,r:id=rId2}))"
)
spTree = sldMaster.find(
"{http://schemas.openxmlformats.org/presentationml/2006/main}cSld/"
"{http://schemas.openxmlformats.org/presentationml/2006/main}spTree"
)
# ---scoped to spTree descendants; 2147483649 etc. are NOT counted---
assert spTree.max_shape_id == 2
# ---and `_next_shape_id` (max + 1) stays in the safe shape-id range---
assert spTree.max_shape_id + 1 == 3

# fixtures ---------------------------------------------

@pytest.fixture
Expand Down
Loading