Bug 175888 – HTMLTrackElement behavior violates the standard

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Kirill Ovchinnikov]

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.

[Kirill Ovchinnikov]

Created attachment 318881 [details]
Patch

[Eric Carlson]

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.

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Kirill Ovchinnikov]

Created attachment 318983 [details]
Patch

[Kirill Ovchinnikov]

Prepared a new patch with some fixes mentioned in comments and tests' results.

[WebKit Commit Bot]

Comment on attachment 318983 [details]
Patch

Clearing flags on attachment: 318983

Committed r221155: <http://trac.webkit.org/changeset/221155>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34064737>