Bug 72553

Summary: Move readyState from TextTrack to HTMLTrackElement
Product: WebKit Reporter: Anna Cavender <annacc>
Component: MediaAssignee: Anna Cavender <annacc>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dglazkov, eric.carlson, ojan, webkit-bug-importer, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 73027    
Bug Blocks: 43668    
Attachments:
Description Flags
Patch
none
updating tests
none
Patch for landing
none
bringin' back the Reflect eric.carlson: review+

Anna Cavender
Reported 2011-11-16 14:55:15 PST
For some reason readyState has been implemented on TextTrack and it really should be on HTMLTrackElement.
Attachments
Patch (15.46 KB, patch)
2011-11-17 13:13 PST, Anna Cavender 		no flags
DetailsFormatted DiffDiff
updating tests (19.69 KB, patch)
2011-11-17 17:56 PST, Anna Cavender 		no flags
DetailsFormatted DiffDiff
Patch for landing (20.24 KB, patch)
2011-11-22 10:58 PST, Anna Cavender 		no flags
DetailsFormatted DiffDiff
bringin' back the Reflect (20.48 KB, patch)
2011-11-23 11:07 PST, Anna Cavender 		eric.carlson: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2011-11-17 12:39:34 PST
<rdar://problem/10464479>
Anna Cavender
Comment 2 2011-11-17 13:13:27 PST
Created attachment 115662 [details] Patch
WebKit Review Bot
Comment 3 2011-11-17 17:22:51 PST
Comment on attachment 115662 [details] Patch Attachment 115662 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10312437 New failing tests: media/track/track-constants.html media/track/track-add-track.html
Anna Cavender
Comment 4 2011-11-17 17:56:51 PST
Created attachment 115717 [details] updating tests oops, forgot to update some other tests
Anna Cavender
Comment 5 2011-11-21 09:53:03 PST
Ping. This patch is ready for review.
Eric Carlson
Comment 6 2011-11-21 22:32:42 PST
Comment on attachment 115717 [details] updating tests View in context: https://bugs.webkit.org/attachment.cgi?id=115717&action=review > Source/WebCore/html/LoadableTextTrack.cpp:67 > + m_trackElement->setReadyState(HTMLTrackElement::LOADING); m_trackElement will be NULL if clearClient() has been called. > Source/WebCore/html/LoadableTextTrack.cpp:103 > + m_trackElement->setReadyState(HTMLTrackElement::LOADING); Dittol
Anna Cavender
Comment 7 2011-11-22 10:58:41 PST
Created attachment 116249 [details] Patch for landing
WebKit Review Bot
Comment 8 2011-11-23 01:25:56 PST
Comment on attachment 116249 [details] Patch for landing Clearing flags on attachment: 116249 Committed r101057: <http://trac.webkit.org/changeset/101057>
WebKit Review Bot
Comment 9 2011-11-23 01:26:01 PST
All reviewed patches have been landed. Closing bug.
Anna Cavender
Comment 10 2011-11-23 10:44:09 PST
Looks like this patch caused some build errors on windows. http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Win%20Builder/builds/16732/steps/compile/logs/stdio I suspect it had to do with my attempt to remove the [Reflect]. I'll add it back and we'll try again.
Anna Cavender
Comment 11 2011-11-23 11:07:43 PST
Created attachment 116384 [details] bringin' back the Reflect
Eric Carlson
Comment 12 2011-11-23 18:08:16 PST
Comment on attachment 116384 [details] bringin' back the Reflect View in context: https://bugs.webkit.org/attachment.cgi?id=116384&action=review > Source/WebCore/html/HTMLTrackElement.h:57 > + enum ReadyState { NONE = 0, LOADING = 1, LOADED = 2, HTML_ERROR = 3 }; Although we used "HTML_ERROR" before, I don't think it is a good choice because "HTML" is too generic. Maybe "TRACK_ERROR" instead?
Anna Cavender
Comment 13 2011-11-27 13:28:34 PST
Thanks Eric, I'll change that before landing.
Anna Cavender
Comment 14 2011-11-27 15:26:17 PST
Committed r101213: <http://trac.webkit.org/changeset/101213>
Note You need to log in before you can comment on or make changes to this bug.