Bug 175888 - HTMLTrackElement behavior violates the standard
Summary: HTMLTrackElement behavior violates the standard
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified All
: P2 Minor
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-08-23 09:50 PDT by Kirill Ovchinnikov
Modified: 2017-08-24 12:45 PDT (History)
6 users (show)

See Also:


Attachments
Updated `layout test` to check described issues (3.34 KB, text/html)
2017-08-23 09:50 PDT, Kirill Ovchinnikov
no flags Details
Patch (9.63 KB, patch)
2017-08-23 10:01 PDT, Kirill Ovchinnikov
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-elcapitan (1.14 MB, application/zip)
2017-08-23 11:07 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (1.28 MB, application/zip)
2017-08-23 11:14 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews112 for mac-elcapitan (1.89 MB, application/zip)
2017-08-23 11:21 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews126 for ios-simulator-wk2 (1.16 MB, application/zip)
2017-08-23 12:46 PDT, Build Bot
no flags Details
Patch (10.71 KB, patch)
2017-08-24 06:16 PDT, Kirill Ovchinnikov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kirill Ovchinnikov 2017-08-23 09:50:24 PDT
Created attachment 318878 [details]
Updated `layout test` to check described issues

- on every TextTrack.cues() and TextTrack.active_cues call the same  the same TextTrackCueList object must be returned each time.
(like the standard requires: https://html.spec.whatwg.org/multipage/media.html#text-track-api:texttrack-22) - now WebKit creates a new object after setting track's src to the empty value

- 'error' event should be fired if the new URL is empty - now WebKit just ignores this.
(https://html.spec.whatwg.org/multipage/media.html#sourcing-out-of-band-text-tracks:event-track-error)

- WebKit now clears TextTrack's cues after 'src' attribute change (see https://bugs.webkit.org/show_bug.cgi?id=175031 for details), but the cues list is also cleared if the new URL is equal to the previous URL (in this case the URL is not really 'changed'), and because of that, if the same URL is set twice, the cue list would be cleared, which doesn't seem like the behavior one would expect.
Comment 1 Kirill Ovchinnikov 2017-08-23 10:01:21 PDT
Created attachment 318881 [details]
Patch
Comment 2 Eric Carlson 2017-08-23 10:35:41 PDT
Comment on attachment 318881 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=318881&action=review

r=me with a few minor suggestions.

Thank you!

> Source/WebCore/html/track/TextTrackCueList.cpp:109
> +void TextTrackCueList::removeAll()

Nit: I think "clear" would be a slightly better name than "removeAll"

> LayoutTests/media/track/text-track-src-change.html:33
>                          cues = testTrack.track.cues;
> +                        testExpected("cues === testTrack.track.cues", true);

This test can't fail. Did you mean to delete "cues = testTrack.track.cues;"?

> LayoutTests/media/track/text-track-src-change.html:51
> +                        errorEventTimer = setTimeout(loadedTrackTest(), 100);

This timeout is too short, it will fire before the error event on heavily loaded machines (e.g. on the bots). I would set it to a much larger value, say 10 seconds.
Comment 3 Build Bot 2017-08-23 11:07:45 PDT
Comment on attachment 318881 [details]
Patch

Attachment 318881 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4369302

New failing tests:
http/tests/security/text-track-crossorigin.html
Comment 4 Build Bot 2017-08-23 11:07:47 PDT
Created attachment 318890 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 5 Build Bot 2017-08-23 11:14:13 PDT
Comment on attachment 318881 [details]
Patch

Attachment 318881 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4369314

New failing tests:
http/tests/security/text-track-crossorigin.html
Comment 6 Build Bot 2017-08-23 11:14:15 PDT
Created attachment 318891 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 7 Build Bot 2017-08-23 11:21:28 PDT
Comment on attachment 318881 [details]
Patch

Attachment 318881 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4369313

New failing tests:
http/tests/security/text-track-crossorigin.html
Comment 8 Build Bot 2017-08-23 11:21:29 PDT
Created attachment 318892 [details]
Archive of layout-test-results from ews112 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 9 Build Bot 2017-08-23 12:46:41 PDT
Comment on attachment 318881 [details]
Patch

Attachment 318881 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4369666

New failing tests:
http/tests/security/text-track-crossorigin.html
Comment 10 Build Bot 2017-08-23 12:46:43 PDT
Created attachment 318903 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 11 Kirill Ovchinnikov 2017-08-24 06:16:19 PDT
Created attachment 318983 [details]
Patch
Comment 12 Kirill Ovchinnikov 2017-08-24 06:17:10 PDT
Prepared a new patch with some fixes mentioned in comments and tests' results.
Comment 13 WebKit Commit Bot 2017-08-24 12:44:24 PDT
Comment on attachment 318983 [details]
Patch

Clearing flags on attachment: 318983

Committed r221155: <http://trac.webkit.org/changeset/221155>
Comment 14 WebKit Commit Bot 2017-08-24 12:44:26 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Radar WebKit Bug Importer 2017-08-24 12:45:39 PDT
<rdar://problem/34064737>