The TextTrackCue constructor has been changed to: [Constructor(double startTime, double endTime, DOMString text)] http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#texttrackcue
Created attachment 159863 [details] Patch
Created attachment 159967 [details] adding check for NaN
Comment on attachment 159967 [details] adding check for NaN View in context: https://bugs.webkit.org/attachment.cgi?id=159967&action=review > Source/WebCore/html/track/TextTrackCue.cpp:92 > + : m_id(emptyString()) I don't think we need to distinguishable between a null and an empty string so this should be unnecessary. > Source/WebCore/html/track/TextTrackCue.h:93 > + void parseSettings(const String&); parseSettings was OK as a private method name but not so much as public method name because, for example, we don't necessarily need to parse the settings string immediately (although we do now) . The spec talks about "WebVTT cue settings" so how about "setCueSettings" instead?
Comment on attachment 159967 [details] adding check for NaN View in context: https://bugs.webkit.org/attachment.cgi?id=159967&action=review Thanks! >> Source/WebCore/html/track/TextTrackCue.cpp:92 >> + : m_id(emptyString()) > > I don't think we need to distinguishable between a null and an empty string so this should be unnecessary. OK. Gone. >> Source/WebCore/html/track/TextTrackCue.h:93 >> + void parseSettings(const String&); > > parseSettings was OK as a private method name but not so much as public method name because, for example, we don't necessarily need to parse the settings string immediately (although we do now) . The spec talks about "WebVTT cue settings" so how about "setCueSettings" instead? Done.
Created attachment 159993 [details] Patch for landing
Comment on attachment 159993 [details] Patch for landing Clearing flags on attachment: 159993 Committed r126350: <http://trac.webkit.org/changeset/126350>
All reviewed patches have been landed. Closing bug.