hasLongWebVTTIdentifier needs to return true even for WebVTT Identifiers that are not long (exactly 6 chars when they match "WEBVTT").
Created attachment 113246 [details] Patch
Comment on attachment 113246 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113246&action=review > Source/WebCore/ChangeLog:8 > + No new tests. This is needed to enable other tests, coming soon. OK. I’m going to hold you to that one! > Source/WebCore/html/track/WebVTTParser.cpp:193 > - if (line[position++] != ' ' && line[position++] != '\t') > + if (line[position] != ' ' && line[position] != '\t') What guarantees we are not at the end of the line here? By the way, I think the ++ was OK, but not two different ++. A local variable would have been another way to do it. > Source/WebCore/html/track/WebVTTParser.cpp:201 > - if (line[position++] != ' ' && line[position++] != '\t') > + if (line[position] != ' ' && line[position] != '\t') Same question/comment.
Comment on attachment 113246 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113246&action=review >> Source/WebCore/ChangeLog:8 >> + No new tests. This is needed to enable other tests, coming soon. > > OK. I’m going to hold you to that one! Thanks Darin. >> Source/WebCore/html/track/WebVTTParser.cpp:193 >> + if (line[position] != ' ' && line[position] != '\t') > > What guarantees we are not at the end of the line here? > > By the way, I think the ++ was OK, but not two different ++. A local variable would have been another way to do it. Good point, I'll add a check. I suppose it doesn't matter since the next action is to ignore remaining white space, but it would eliminate one extra check. I'll use a local var like you suggest. >> Source/WebCore/html/track/WebVTTParser.cpp:201 >> + if (line[position] != ' ' && line[position] != '\t') > > Same question/comment. Ditto.
Created attachment 113276 [details] Patch for landing
Comment on attachment 113276 [details] Patch for landing Clearing flags on attachment: 113276 Committed r99039: <http://trac.webkit.org/changeset/99039>
All reviewed patches have been landed. Closing bug.