WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
88583
Update the TextTrackCue Constructor
https://bugs.webkit.org/show_bug.cgi?id=88583
Summary
Update the TextTrackCue Constructor
Victor Carbune
Reported
2012-06-07 15:22:38 PDT
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
Attachments
Patch
(9.94 KB, patch)
2012-08-21 22:44 PDT
,
Anna Cavender
no flags
Details
Formatted Diff
Diff
adding check for NaN
(12.48 KB, patch)
2012-08-22 11:09 PDT
,
Anna Cavender
no flags
Details
Formatted Diff
Diff
Patch for landing
(12.77 KB, patch)
2012-08-22 12:40 PDT
,
Anna Cavender
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Anna Cavender
Comment 1
2012-08-21 22:44:09 PDT
Created
attachment 159863
[details]
Patch
Anna Cavender
Comment 2
2012-08-22 11:09:34 PDT
Created
attachment 159967
[details]
adding check for NaN
Eric Carlson
Comment 3
2012-08-22 11:35:01 PDT
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?
Anna Cavender
Comment 4
2012-08-22 12:26:58 PDT
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.
Anna Cavender
Comment 5
2012-08-22 12:40:17 PDT
Created
attachment 159993
[details]
Patch for landing
WebKit Review Bot
Comment 6
2012-08-22 14:24:07 PDT
Comment on
attachment 159993
[details]
Patch for landing Clearing flags on attachment: 159993 Committed
r126350
: <
http://trac.webkit.org/changeset/126350
>
WebKit Review Bot
Comment 7
2012-08-22 14:24:11 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug