diff --git a/src/pptx/oxml/shapes/groupshape.py b/src/pptx/oxml/shapes/groupshape.py index 4687b5511..33c94d49a 100644 --- a/src/pptx/oxml/shapes/groupshape.py +++ b/src/pptx/oxml/shapes/groupshape.py @@ -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 `` (or + ``) element. Scoped strictly to the shape tree because OOXML + uses sibling element-types (notably `/` + 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 diff --git a/tests/oxml/shapes/test_groupshape.py b/tests/oxml/shapes/test_groupshape.py index 6884b06cd..3db308b95 100644 --- a/tests/oxml/shapes/test_groupshape.py +++ b/tests/oxml/shapes/test_groupshape.py @@ -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: `` lives as a + # sibling of `` 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