Timestamps like this should not be allowed: 00:04:01.000-->00:03:00.500 They should instead be required to have at least one whitespace character between the startTime/endTime and the arrow: 00:04:01.000 --> 00:03:00.500
Created attachment 111332 [details] Patch
Attachment 111332 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 2 Updating OpenSource From git://git.webkit.org/WebKit c0d4f37..d10d208 master -> origin/master M Source/WebCore/ChangeLog M Source/WebCore/html/HTMLKeygenElement.h M Source/WebCore/html/HTMLKeygenElement.cpp r97658 = c627b94a38756dd0cde4d64c2288f36bff87ce20 (refs/remotes/trunk) A LayoutTests/fast/mutation/observe-attributes-expected.txt A LayoutTests/fast/mutation/observe-attributes.html M LayoutTests/ChangeLog M Source/WebCore/dom/MutationRecord.idl M Source/WebCore/dom/MutationRecord.cpp M Source/WebCore/dom/Element.cpp M Source/WebCore/dom/Node.h M Source/WebCore/dom/WebKitMutationObserver.h M Source/WebCore/dom/NodeRareData.h M Source/WebCore/dom/MutationRecord.h M Source/WebCore/dom/Node.cpp M Source/WebCore/dom/WebKitMutationObserver.cpp M Source/WebCore/ChangeLog M Source/WebCore/bindings/v8/V8Proxy.cpp M Source/WebCore/WebCore.xcodeproj/project.pbxproj r97659 = d10d208ef8dc738eb8d45c331840c61c2c779498 (refs/remotes/trunk) First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/trunk. Updating chromium port dependencies using gclient... Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again. Re-trying 'depot_tools/gclient sync' Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again. Re-trying 'depot_tools/gclient sync' Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again. Error: 'depot_tools/gclient sync' failed 3 tries and returned 256 at Tools/Scripts/update-webkit-chromium line 107. Re-trying 'depot_tools/gclient sync' No such file or directory at Tools/Scripts/update-webkit line 104. If any of these errors are false positives, please file a bug against check-webkit-style.
Hmm, the spec at http://www.whatwg.org/specs/web-apps/current-work/webvtt.html currently states only the U+0020 SPACE character as a valid separator between the "-->" string and the timestamps. This accepts other white space as separators, too, in particular isASpace() accepts also U+0009 CHARACTER TABULATION (tab), U+000A LINE FEED (LF), U+000C FORM FEED (FF), and U+000D CARRIAGE RETURN. I've just registered a bug on the spec to allow the TAB character as a separator, see http://www.w3.org/Bugs/Public/show_bug.cgi?id=14487. But I do not believe the newline characters should be allowed.
I agree line terminators should not be allowed, but collectTimingsAndSettings() is only ever handed a single line (so it will not have any line terminators). Perhaps the use of isASpace() is confusing here, would it be more clear to just check for ' ' and/or '\t' instead of using isASpace()?
Created attachment 111538 [details] Patch How about we make sure that at least one space separates the time from the --> for now? We can always update if more whitespace characters are allowed in the future.
Works for me. :-)
Created attachment 111543 [details] adding check for tabs Ha, one of our tests (tc008-timings-no-hour-errors.vtt) checks for tabs, so at least WE think tabs should be allowed. Adding that here.
Comment on attachment 111543 [details] adding check for tabs Clearing flags on attachment: 111543 Committed r97883: <http://trac.webkit.org/changeset/97883>
All reviewed patches have been landed. Closing bug.
As of a few hours ago, the spec now also tolerates tabs, see http://www.w3.org/Bugs/Public/show_bug.cgi?id=14487 . We should add this.
Great! We too allow both spaces and tabs, so looks like we are good to go. A small note: the parser in the spec currently allows *0 or more* space or tab characters, not *1 or more* space or tab characters as specified in Ian's change. Our implementation now enforces at least 1 space or tab character (so that [time]-->[time] without any spaces or tabs is not allowed).