OCTO-11629

docs/changelog.rst

@@ -10,6 +10,15 @@ Changelog
     dropped, ``CaptionReadNoCaptions`` is raised. With
     ``ignore_timing_errors=False``, clamped cues no longer fail the
     start-ordering check. Zero and positive shifts are unchanged.
+  - Fix ``WebVTTReader`` not closing a cue with no text at the blank line
+    that ends it, so the next block was read as that cue's text: a following
+    cue's ID appeared above its caption, and a ``NOTE`` comment, ``STYLE``
+    block or ``REGION`` block became a caption at the empty cue's time; these
+    blocks are now skipped. Empty cues are still dropped; the
+    following ID now counts towards the duplicate-ID warning. Text after an
+    empty cue that is not part of a cue is now ignored, so a file whose only
+    text sits there raises ``CaptionReadNoCaptions``. Files without empty
+    cues are unchanged.
 
 2.3.13
 ^^^^^^

pycaption/webvtt/reader.py

@@ -258,12 +258,10 @@ def _parse_line(self, line, line_index, state, captions):
             state.pending_id = None
             return
 
-        if line == "" and state.found_timing and state.nodes:
+        if line == "" and state.found_timing:
             self._finalize_cue(state, captions)
             return
 
-        if not state.found_timing or line == "":
-            return
         if state.nodes:
             state.nodes.append(CaptionNode.create_break())
         state.nodes.extend(self._parse_cue_text(line, state.open_tags))
@@ -317,10 +315,10 @@ def _consume_pending_id(state, line_index):
         state.seen_ids.add(cue_id)
 
     def _finalize_cue(self, state, captions):
-        """Close the current cue, append it to captions, and reset state
-        for the next cue."""
+        """Close the current cue, append it to captions unless it is dropped
+        or empty, and reset state for the next cue."""
         state.found_timing = False
-        if not state.drop_cue:
+        if not state.drop_cue and state.nodes:
             captions.append(
                 self._build_cue(
                     state.nodes,

tests/conftest.py

@@ -219,7 +219,16 @@
     sample_webvtt_double_br,
     sample_webvtt_empty,
     sample_webvtt_empty_cue,
+    sample_webvtt_empty_cue_multiple_blanks,
+    sample_webvtt_empty_cue_only_orphan_text,
     sample_webvtt_empty_cue_output,
+    sample_webvtt_empty_cue_then_duplicate_id,
+    sample_webvtt_empty_cue_then_id,
+    sample_webvtt_empty_cue_then_note,
+    sample_webvtt_empty_cue_then_orphan_text,
+    sample_webvtt_empty_cue_then_region,
+    sample_webvtt_empty_cue_then_style,
+    sample_webvtt_empty_last_cue,
     sample_webvtt_from_dfxp,
     sample_webvtt_from_dfxp_with_positioning_and_style,
     sample_webvtt_from_dfxp_with_style,

tests/fixtures/webvtt.py

@@ -343,6 +343,124 @@ def sample_webvtt_empty_cue():
 """
 
 
+@pytest.fixture(scope="session")
+def sample_webvtt_empty_cue_then_id():
+    return """WEBVTT
+
+cue-1
+00:00.000 --> 00:02.000
+
+cue-2
+00:04.000 --> 00:05.000
+C
+"""
+
+
+@pytest.fixture(scope="session")
+def sample_webvtt_empty_cue_then_note():
+    return """WEBVTT
+
+00:00.000 --> 00:02.000
+
+NOTE hello
+
+00:04.000 --> 00:05.000
+C
+"""
+
+
+@pytest.fixture(scope="session")
+def sample_webvtt_empty_cue_then_style():
+    return """WEBVTT
+
+00:00.000 --> 00:02.000
+
+STYLE
+::cue { color: red }
+
+00:04.000 --> 00:05.000
+C
+"""
+
+
+@pytest.fixture(scope="session")
+def sample_webvtt_empty_cue_then_region():
+    return """WEBVTT
+
+00:00.000 --> 00:02.000
+
+REGION
+id:r1
+
+00:04.000 --> 00:05.000
+C
+"""
+
+
+@pytest.fixture(scope="session")
+def sample_webvtt_empty_cue_multiple_blanks():
+    return """WEBVTT
+
+00:00.000 --> 00:02.000
+
+
+
+cue-2
+00:04.000 --> 00:05.000
+C
+"""
+
+
+@pytest.fixture(scope="session")
+def sample_webvtt_empty_last_cue():
+    return """WEBVTT
+
+cue-1
+00:00.000 --> 00:02.000
+A
+
+cue-2
+00:03.000 --> 00:04.000
+"""
+
+
+@pytest.fixture(scope="session")
+def sample_webvtt_empty_cue_then_duplicate_id():
+    return """WEBVTT
+
+cue-1
+00:00.000 --> 00:02.000
+
+cue-1
+00:04.000 --> 00:05.000
+C
+"""
+
+
+@pytest.fixture(scope="session")
+def sample_webvtt_empty_cue_then_orphan_text():
+    return """WEBVTT
+
+00:00.000 --> 00:02.000
+
+hello
+world
+
+00:04.000 --> 00:05.000
+C
+"""
+
+
+@pytest.fixture(scope="session")
+def sample_webvtt_empty_cue_only_orphan_text():
+    return """WEBVTT
+
+00:00.000 --> 00:02.000
+
+hello
+"""
+
+
 @pytest.fixture(scope="session")
 def sample_webvtt_multi_lang_en():
     return """WEBVTT

tests/test_webvtt.py

@@ -298,6 +298,75 @@ def test_zero_start(self, sample_webvtt_last_cue_zero_start):
     def test_webvtt_empty_cue(self, sample_webvtt_empty_cue):
         assert 1 == len(self.reader.read(sample_webvtt_empty_cue).get_captions("en-US"))
 
+    @pytest.mark.parametrize("ignore_timing_errors", [True, False])
+    @pytest.mark.parametrize(
+        "fixture_name",
+        [
+            "sample_webvtt_empty_cue_then_id",
+            "sample_webvtt_empty_cue_then_note",
+            "sample_webvtt_empty_cue_then_style",
+            "sample_webvtt_empty_cue_then_region",
+            "sample_webvtt_empty_cue_multiple_blanks",
+        ],
+    )
+    def test_blank_line_closes_empty_cue(
+        self, request, fixture_name, ignore_timing_errors
+    ):
+        captions = WebVTTReader(ignore_timing_errors=ignore_timing_errors).read(
+            request.getfixturevalue(fixture_name)
+        )
+
+        assert [
+            (c.start, c.end, c.get_text()) for c in captions.get_captions("en-US")
+        ] == [(4000000, 5000000, "C")]
+
+    def test_empty_last_cue_is_dropped(self, sample_webvtt_empty_last_cue):
+        captions = self.reader.read(sample_webvtt_empty_last_cue)
+
+        assert [
+            (c.start, c.end, c.get_text()) for c in captions.get_captions("en-US")
+        ] == [(0, 2000000, "A")]
+
+    @pytest.mark.parametrize("ignore_timing_errors", [True, False])
+    def test_empty_cue_id_counts_for_duplicate_warning(
+        self, sample_webvtt_empty_cue_then_duplicate_id, ignore_timing_errors
+    ):
+        reader = WebVTTReader(ignore_timing_errors=ignore_timing_errors)
+        with pytest.warns(CaptionReadWarning, match="Duplicate cue identifier 'cue-1'"):
+            captions = reader.read(sample_webvtt_empty_cue_then_duplicate_id)
+
+        assert [c.get_text() for c in captions.get_captions("en-US")] == ["C"]
+
+    def test_dropped_empty_cue_before_zero_is_closed(self):
+        captions = WebVTTReader(time_shift_milliseconds=-2000).read(
+            "WEBVTT\n\n00:00.000 --> 00:01.000\n\n"
+            "cue-2\n00:03.000 --> 00:04.000\nC\n"
+        )
+
+        assert [
+            (c.start, c.end, c.get_text()) for c in captions.get_captions("en-US")
+        ] == [(1000000, 2000000, "C")]
+
+    @pytest.mark.parametrize("ignore_timing_errors", [True, False])
+    def test_orphan_text_after_empty_cue_is_ignored(
+        self, sample_webvtt_empty_cue_then_orphan_text, ignore_timing_errors
+    ):
+        captions = WebVTTReader(ignore_timing_errors=ignore_timing_errors).read(
+            sample_webvtt_empty_cue_then_orphan_text
+        )
+
+        assert [
+            (c.start, c.end, c.get_text()) for c in captions.get_captions("en-US")
+        ] == [(4000000, 5000000, "C")]
+
+    @pytest.mark.parametrize("ignore_timing_errors", [True, False])
+    def test_only_orphan_text_after_empty_cue_raises_no_captions(
+        self, sample_webvtt_empty_cue_only_orphan_text, ignore_timing_errors
+    ):
+        reader = WebVTTReader(ignore_timing_errors=ignore_timing_errors)
+        with pytest.raises(CaptionReadNoCaptions):
+            reader.read(sample_webvtt_empty_cue_only_orphan_text)
+
 
 class TestWebVTTWriter:
     def setup_method(self):

tests/test_webvtt_conversion.py

@@ -110,6 +110,30 @@ def test_output_timestamps(self, sample_webvtt_timestamps):
         assert re.match(expected_timestamp_line_pattern, results[5])
 
 
+class TestWebVTTEmptyCueConversion:
+    @pytest.mark.parametrize(
+        "writer", [WebVTTWriter, SRTWriter, DFXPWriter, SAMIWriter]
+    )
+    @pytest.mark.parametrize("ignore_timing_errors", [True, False])
+    @pytest.mark.parametrize(
+        "fixture_name, leaked_text",
+        [
+            ("sample_webvtt_empty_cue_then_id", "cue-2"),
+            ("sample_webvtt_empty_cue_then_note", "hello"),
+            ("sample_webvtt_empty_cue_multiple_blanks", "cue-2"),
+        ],
+    )
+    def test_no_leak_after_empty_cue(
+        self, request, fixture_name, leaked_text, ignore_timing_errors, writer
+    ):
+        caption_set = WebVTTReader(ignore_timing_errors=ignore_timing_errors).read(
+            request.getfixturevalue(fixture_name)
+        )
+        results = writer().write(caption_set)
+
+        assert leaked_text not in results
+
+
 #     # TODO: Write a test that includes a WebVTT file with style tags
 #     # That will fail because the styles used in the cues are not tracked.