Merge pull request #6300 from raygard/comment_correct_placement

Only write GIF comments at the beginning of the file

Author: radarhere

Tests/images/second_frame_comment.gif

@@ -837,6 +837,50 @@ def test_read_multiple_comment_blocks():
         assert im.info["comment"] == b"Test comment 1\nTest comment 2"
 
 
+def test_empty_string_comment(tmp_path):
+    out = str(tmp_path / "temp.gif")
+    with Image.open("Tests/images/chi.gif") as im:
+        assert "comment" in im.info
+
+        # Empty string comment should suppress existing comment
+        im.save(out, save_all=True, comment="")
+
+    with Image.open(out) as reread:
+        for frame in ImageSequence.Iterator(reread):
+            assert "comment" not in frame.info
+
+
+def test_retain_comment_in_subsequent_frames(tmp_path):
+    # Test that a comment block at the beginning is kept
+    with Image.open("Tests/images/chi.gif") as im:
+        for frame in ImageSequence.Iterator(im):
+            assert frame.info["comment"] == b"Created with GIMP"
+
+    with Image.open("Tests/images/second_frame_comment.gif") as im:
+        assert "comment" not in im.info
+
+        # Test that a comment in the middle is read
+        im.seek(1)
+        assert im.info["comment"] == b"Comment in the second frame"
+
+        # Test that it is still present in a later frame
+        im.seek(2)
+        assert im.info["comment"] == b"Comment in the second frame"
+
+        # Test that rewinding removes the comment
+        im.seek(0)
+        assert "comment" not in im.info
+
+    # Test that a saved image keeps the comment
+    out = str(tmp_path / "temp.gif")
+    with Image.open("Tests/images/dispose_prev.gif") as im:
+        im.save(out, save_all=True, comment="Test")
+
+    with Image.open(out) as reread:
+        for frame in ImageSequence.Iterator(reread):
+            assert frame.info["comment"] == b"Test"
+
+
 def test_version(tmp_path):
     out = str(tmp_path / "temp.gif")
 

Tests/test_file_gif.py

@@ -163,6 +163,8 @@ def _seek(self, frame, update_image=True):
             self.__frame = -1
             self._fp.seek(self.__rewind)
             self.disposal_method = 0
+            if "comment" in self.info:
+                del self.info["comment"]
         else:
             # ensure that the previous frame was loaded
             if self.tile and update_image:
@@ -230,7 +232,7 @@ def _seek(self, frame, update_image=True):
                     #
                     comment = b""
 
-                    # Collect one comment block
+                    # Read this comment block
                     while block:
                         comment += block
                         block = self.data()
@@ -395,7 +397,9 @@ def _rgb(color):
                 )
             ]
 
-        for k in ["duration", "comment", "extension", "loop"]:
+        if info.get("comment"):
+            self.info["comment"] = info["comment"]
+        for k in ["duration", "extension", "loop"]:
             if k in info:
                 self.info[k] = info[k]
             elif k in self.info:
@@ -712,15 +716,6 @@ def _write_local_header(fp, im, offset, flags):
             + o8(0)
         )
 
-    if "comment" in im.encoderinfo and 1 <= len(im.encoderinfo["comment"]):
-        fp.write(b"!" + o8(254))  # extension intro
-        comment = im.encoderinfo["comment"]
-        if isinstance(comment, str):
-            comment = comment.encode()
-        for i in range(0, len(comment), 255):
-            subblock = comment[i : i + 255]
-            fp.write(o8(len(subblock)) + subblock)
-        fp.write(o8(0))
     if "loop" in im.encoderinfo:
         number_of_loops = im.encoderinfo["loop"]
         fp.write(
@@ -925,7 +920,7 @@ def _get_global_header(im, info):
     palette_bytes = _get_palette_bytes(im)
     color_table_size = _get_color_table_size(palette_bytes)
 
-    return [
+    header = [
         b"GIF"  # signature
         + version  # version
         + o16(im.size[0])  # canvas width
@@ -938,6 +933,19 @@ def _get_global_header(im, info):
         # Global Color Table
         _get_header_palette(palette_bytes),
     ]
+    if info.get("comment"):
+        comment_block = b"!" + o8(254)  # extension intro
+
+        comment = info["comment"]
+        if isinstance(comment, str):
+            comment = comment.encode()
+        for i in range(0, len(comment), 255):
+            subblock = comment[i : i + 255]
+            comment_block += o8(len(subblock)) + subblock
+
+        comment_block += o8(0)
+        header.append(comment_block)
+    return header
 
 
 def _write_frame_data(fp, im_frame, offset, params):