864765 - SRT files starting with blank line throw exception when parsing

Status: RESOLVED FIXED

(Webmaker Graveyard :: popcorn.js, defect)

Description

[Chris DeCairos (:cade)]

On GitHub, arnaudbreton wrote:

"Hi,

I've just started to use PopcornJS which is awesome to handle these many video providers!

I'm working with subtitles, in SRT format, from Coursera (http://coursera.org) and their subtitles start with a blank line.

When I parse this file, it crashes, line 63 (idx = time[1].indexOf( " " )).

Thanks for your help."

I have confirmed the bug. I'm actually surprised that this hasn't come to light sooner.

Comment 1

[Eduardo Veiga]

Hi! I started working on this and was able to fix it, now I only need to add some tests. Could someone please assign it to me? (looks like I can't)
Thanks!

Comment 2

[Chris DeCairos (:cade)]

Sounds great Eduardo! I've assigned the bug to you!

Comment 3

[Eduardo Veiga]

I created a pull request for this: https://github.com/mozilla/popcorn-js/pull/375

Comment 4

[Scott [:thecount] Downe]

Hey Eduardo,

I took a quick look at this, and it looks pretty good.

To put a patch into review:

1. Click on the add attachment link above.
2. Inside that page, click "paste text as attachment" under the File input.
3. Now that file input is text input, paste in the pull request url you have in comment 3.
4. Also paste that pull request url into description.
5. Under flgas > review, set it to "?".
6. Under requestee, start typing ":thecount", it should autocomplete to my name and email.
7. Hit submit.

Comment 5

[Eduardo Veiga]

Attachment: https://github.com/mozilla/popcorn-js/pull/375

Comment 6

[Scott [:thecount] Downe]

Comment on attachment 8369453 [details] [review]
https://github.com/mozilla/popcorn-js/pull/375

Awesome work.

Works well, r+ cept do you mind merging the commits into one? r+ with that :)

Comment 7

[Eduardo Veiga]

Done!

Comment 8

[[github robot]]

Commit pushed to master at https://github.com/mozilla/popcorn-js

https://github.com/mozilla/popcorn-js/commit/3325b800d60202625d2fc2f148e92d2e5ddaa8ff
Bug 864765 - Correctly parse SRT files starting with blank lines

Comment 9

[Eduardo Veiga]

I see this issue is still open, should I close it?

Comment 10

[Scott [:thecount] Downe]

Ah yes, forgot.  Thanks for the reminder.