Bug 88317 - addTextTrack should set track mode to HIDDEN
Summary: addTextTrack should set track mode to HIDDEN
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Victor Carbune
URL:
Keywords:
Depends on:
Blocks: 43668
  Show dependency tree / graph
 
Reported: 2012-06-05 02:38 PDT by Victor Carbune
Modified: 2012-06-07 20:22 PDT (History)
6 users (show)

See Also:

Attachments
Updated code (5.85 KB, patch)
2012-06-05 02:51 PDT, Victor Carbune 		no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cq-01 (625.39 KB, application/zip)
2012-06-05 15:31 PDT, WebKit Review Bot 		no flags Details
Archive of layout-test-results from ec2-cr-linux-04 (774.02 KB, application/zip)
2012-06-05 21:16 PDT, WebKit Review Bot 		no flags Details
Updated patch (6.51 KB, patch)
2012-06-07 15:53 PDT, Victor Carbune 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Victor Carbune 2012-06-05 02:38:11 PDT 
Implicitly, the method addTextTrack should set the text track mode to HIDDEN and readiness state to LOADED.
Comment 1 Victor Carbune 2012-06-05 02:51:27 PDT 
Created attachment 145740 [details]
Updated code
Comment 2 Eric Carlson 2012-06-05 09:50:29 PDT 
Comment on attachment 145740 [details]
Updated code

Good catch, thanks Victor!
Comment 3 WebKit Review Bot 2012-06-05 15:31:52 PDT 
Comment on attachment 145740 [details]
Updated code

Rejecting attachment 145740 [details] from commit-queue.

New failing tests:
media/track/track-text-track-destructor-crash.html
Full output: http://queues.webkit.org/results/12898667
Comment 4 WebKit Review Bot 2012-06-05 15:31:55 PDT 
Created attachment 145880 [details]
Archive of layout-test-results from ec2-cq-01

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: ec2-cq-01  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 5 WebKit Review Bot 2012-06-05 21:16:50 PDT 
Comment on attachment 145740 [details]
Updated code

Attachment 145740 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12907132

New failing tests:
media/track/track-text-track-destructor-crash.html
Comment 6 WebKit Review Bot 2012-06-05 21:16:55 PDT 
Created attachment 145928 [details]
Archive of layout-test-results from ec2-cr-linux-04

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-04  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 7 Victor Carbune 2012-06-07 15:53:40 PDT 
Created attachment 146403 [details]
Updated patch
Comment 8 Victor Carbune 2012-06-07 15:56:08 PDT 
Comment on attachment 146403 [details]
Updated patch

The trouble with the failing test was that setMode has side effects and calls configureTextTracks, but the new TextTrack has not yet been appended to the TextTrackList.

The simplest fix is to change the order: first add the track and set the parameters right after, since it preserves the spec intention.
Comment 9 WebKit Review Bot 2012-06-07 20:22:11 PDT 
Comment on attachment 146403 [details]
Updated patch

Clearing flags on attachment: 146403

Committed r119792: <http://trac.webkit.org/changeset/119792>
Comment 10 WebKit Review Bot 2012-06-07 20:22:16 PDT 
All reviewed patches have been landed.  Closing bug.