tballison commented on PR #3566:
URL: https://github.com/apache/tika/pull/3566#issuecomment-6741858084
Thank you for the PR. My :robot: has some feedback. Let me know what you (as
a human) think.
```
. The bug it says it fixes is still there. With the guard, the bad timing
line falls through to text(line). When no div is open, text() opens <div
class="cue"> with no timing attributes, which is exactly what the PR says it
removes. All that changes is
that the junk line 00:61.000 --> 00:62.000 now ends up in the extracted
content too. The test doesn't notice because:
- cues() collects <p> contents, not divs.
- The last assertion only counts divs that have data-start.
- The test comment ("no attribute-less cue div is emitted") contradicts
what its own first assertion expects.
2. New problem: text gets credited to the previous cue. write() doesn't
close the div on a blank line while block == CUE (SubtitleParser.java:297). So
for
00:01.000 --> 00:02.000
first cue
00:61.000 --> 00:62.000
bad block text
- Main: closeDiv() runs, and "bad block text" goes into its own div with
no timing.
- PR: the timing line and "bad block text" become a new <p> inside the
first cue's div, so they inherit data-start=00:00:01.000. That timing is wrong
for that text.
3. The description gets the spec and the example wrong.
- In 00:61.000, the 61 is seconds, not minutes.
- The WebVTT parsing algorithm drops a block whose timings fail to parse.
It doesn't "degrade it to cue text."
Main's current output keeps the text, drops the bad timing string and
starts a fresh div with no timing. That fits Tika's habit of keeping content
and doesn't credit text to the wrong cue.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
