Bug 72553

Summary:

Move readyState from TextTrack to HTMLTrackElement

Product:

WebKit

Reporter:

Anna Cavender <annacc>

Component:

Media

Assignee:

Anna Cavender <annacc>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, dglazkov, eric.carlson, ojan, webkit-bug-importer, webkit.review.bot

Priority:

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	Details Formatted Diff Diff
updating tests (19.69 KB, patch) 2011-11-17 17:56 PST, Anna Cavender	no flags	Details Formatted Diff Diff
Patch for landing (20.24 KB, patch) 2011-11-22 10:58 PST, Anna Cavender	no flags	Details Formatted Diff Diff
bringin' back the Reflect (20.48 KB, patch) 2011-11-23 11:07 PST, Anna Cavender	eric.carlson: review+	Details Formatted Diff Diff
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.