Closed Bug 1328966 Opened 3 years ago Closed 3 years ago

VTT files with CR line endings do not parse

Categories

(Core :: Audio/Video: Playback, defect)

50 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
platform-rel --- -
firefox50 --- wontfix
firefox51 --- wontfix
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: me, Assigned: rillian)

Details

(Whiteboard: [platform-rel-Vimeo])

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_2) AppleWebKit/602.3.12 (KHTML, like Gecko) Version/10.0.2 Safari/602.3.12

Steps to reproduce:

Try to play a video with subtitles from a VTT file that has CR line endings. See the Spanish subtitles on this video: https://vimeo.com/198237283/5f5f685091


Actual results:

The subtitles don't show. You can see in the network inspector that the file was properly loaded and if you inspect the <track> you will see that the track.cues CueList is empty.


Expected results:

The subtitles should show.
Summary: VTT files with CR line endings do not load → VTT files with CR line endings do not parse
Component: Untriaged → Audio/Video: Playback
Product: Firefox → Core
platform-rel: --- → ?
Whiteboard: [platform-rel-Vimeo]
Hi Ralph -- Can you take a look at this?  Can you reproduce?  If so, can you take this?  Thanks!
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(giles)
Blake: Is there someone on your team that can look into this?
Flags: needinfo?(bwu)
I was able to look at this a bit too. The spec has been updated to allow all three line ending variations, so we need to modify vtt.jsm. I have a potential started working on tests, but haven't verified. If someone else wants to take over, feel free. Otherwise I'll continue tomorrow.
Flags: needinfo?(giles)
(In reply to Chris Pearce (:cpearce) from comment #2)
> Blake: Is there someone on your team that can look into this?
Yeah. I have cc-ed Benjamin in this bug. But since Ralph has some ideas already, it would be better to have him finish it continuously.  

Ralph, 
Thanks!
Flags: needinfo?(bwu)
Assignee: nobody → giles
Comment on attachment 8826776 [details]
Bug 1328966 - Accept \r as a valid line ending.

https://reviewboard.mozilla.org/r/104654/#review105560
Attachment #8826776 - Flags: review?(alwu) → review+
Pushed by rgiles@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b7f25749589e
Accept \r as a valid line ending. r=alwu
https://hg.mozilla.org/mozilla-central/rev/b7f25749589e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Ralph, can we uplift this VTT fix to Aurora 52 since it fixes a web compat issue? It's probably too late to uplift to Beta 51, which will be released next week.
platform-rel: ? → -
Flags: needinfo?(giles)
I guess 52 is affected too.
Comment on attachment 8826776 [details]
Bug 1328966 - Accept \r as a valid line ending.

I think uplift is a good idea.

Approval Request Comment
[Feature/Bug causing the regression]: WebVTT subtitles
[User impact if declined]: webcompat issue: Some video subtitles will not display.
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes

[Needs manual test from QE? If yes, steps to reproduce]: Would be good to confirm. Visit https://vimeo.com/198237283/5f5f685091, press play, and select Spanish (Español) subtitles from the 'CC' player control. Confirm subtitles appear starting 5 seconds into the video.

[List of other uplifts needed for the feature/fix]: None.

[Is the change risky?]: No.
[Why is the change risky/not risky?]:  It is a single-line change which should not affect parsing of files we handled without the change. If it causes problems, it is easy to revert.

[String changes made/needed]: None
Flags: needinfo?(giles)
Attachment #8826776 - Flags: approval-mozilla-aurora?
Comment on attachment 8826776 [details]
Bug 1328966 - Accept \r as a valid line ending.

OK, let's take it then, thanks!
Attachment #8826776 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.