Bug 72553 - Move readyState from TextTrack to HTMLTrackElement
Summary: Move readyState from TextTrack to HTMLTrackElement
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Anna Cavender
URL:
Keywords: InRadar
Depends on: 73027
Blocks: 43668
  Show dependency treegraph
 
Reported: 2011-11-16 14:55 PST by Anna Cavender
Modified: 2011-11-27 15:26 PST (History)
6 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Anna Cavender 2011-11-16 14:55:15 PST
For some reason readyState has been implemented on TextTrack and it really should be on HTMLTrackElement.
Comment 1 Radar WebKit Bug Importer 2011-11-17 12:39:34 PST
<rdar://problem/10464479>
Comment 2 Anna Cavender 2011-11-17 13:13:27 PST
Created attachment 115662 [details]
Patch
Comment 3 WebKit Review Bot 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
Comment 4 Anna Cavender 2011-11-17 17:56:51 PST
Created attachment 115717 [details]
updating tests

oops, forgot to update some other tests
Comment 5 Anna Cavender 2011-11-21 09:53:03 PST
Ping.  This patch is ready for review.
Comment 6 Eric Carlson 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
Comment 7 Anna Cavender 2011-11-22 10:58:41 PST
Created attachment 116249 [details]
Patch for landing
Comment 8 WebKit Review Bot 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>
Comment 9 WebKit Review Bot 2011-11-23 01:26:01 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Anna Cavender 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.
Comment 11 Anna Cavender 2011-11-23 11:07:43 PST
Created attachment 116384 [details]
bringin' back the Reflect
Comment 12 Eric Carlson 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?
Comment 13 Anna Cavender 2011-11-27 13:28:34 PST
Thanks Eric, I'll change that before landing.
Comment 14 Anna Cavender 2011-11-27 15:26:17 PST
Committed r101213: <http://trac.webkit.org/changeset/101213>