VTT files with CR line endings do not parse

RESOLVED FIXED in Firefox 52

Status

()

Core
Audio/Video: Playback
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: Brad Dougherty, Assigned: rillian)

Tracking

50 Branch
mozilla53
Points:
---

Firefox Tracking Flags

(platform-rel -, firefox50 wontfix, firefox51 wontfix, firefox52 fixed, firefox53 fixed)

Details

(Whiteboard: [platform-rel-Vimeo])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

a year ago
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.
(Reporter)

Updated

a year ago
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]
status-firefox50: --- → affected
status-firefox51: --- → ?
status-firefox52: --- → ?
status-firefox53: --- → ?
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)
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
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+

Comment 7

a year ago
Pushed by rgiles@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b7f25749589e
Accept \r as a valid line ending. r=alwu

Comment 8

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b7f25749589e
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox53: ? → fixed
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: ? → -
status-firefox51: ? → wontfix
Flags: needinfo?(giles)
status-firefox50: affected → wontfix
I guess 52 is affected too.
status-firefox52: ? → affected
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+

Comment 13

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/811c33f33507
status-firefox52: affected → fixed
You need to log in before you can comment on or make changes to this bug.