Merge pull request #53 from MHoroszowski/fix/master-shape-id-overflow

fix(oxml): scope CT_GroupShape.max_shape_id to spTree descendants (post-#52 hotfix)

Author: MHoroszowski

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 `<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
 

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: `<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