fix for #5292 - push to filterSubtitleTracks output if kind is captions

[dstreet26]

This PR will...

fix #5292

Are there any points in the code the reviewer needs to double check?

I did not test on Edge, which is part of the comments in filterSubtitleTracks..

So I guess just make sure this is still working on different agents/devices as originally intended?

Checklist

• changes have been done against master branch, and PR does not conflict
• new unit / functional tests have been added (whenever applicable)
  • Can be added to this PR if needed
• API or design changes are documented in API.md
  • N/A

[robwalch]

onTextTrackChanged and toggleTrackModes depend on filterSubtitleTracks and have test coverage.

hls.js/tests/unit/controller/subtitle-track-controller.js

Line 223 in a0da6e7

describe('toggleTrackModes', function () {

The issue is the tests are setup to add two subtitle tracks and no captions tracks:

    const textTrack1 = videoElement.addTextTrack('subtitles', 'English', 'en');
    const textTrack2 = videoElement.addTextTrack('subtitles', 'Swedish', 'se');

We should include a captions track and update the tests so that there is coverage for this issue and we don't repeat the regression in the future.

[dstreet26]

I'm not sure the best way to go about this in this repo so I just wrapped the test...

I added ['subtitles', 'captions'].forEach(function (captionType) { to the top of the test file.

If this is not the right way, I can revert. Like I said, not sure about what to do.

[robwalch]

I added ['subtitles', 'captions'].forEach(function (captionType) { to the top of the test file.

This does get us coverage. I'm going to merge and create a patch. We can circle back to tests that cover a mix of tracks with different characteristics at a later date.

[robwalch]

@dstreet26 v1.3.5 is up.

After merging and cherry picking I updated the tests to just include an additional captions track and test:

    const textTrack1 = videoElement.addTextTrack('subtitles', 'English', 'en');
    const textTrack2 = videoElement.addTextTrack('subtitles', 'Swedish', 'se');
    const textTrack3 = videoElement.addTextTrack(
      'captions',
      'Untitled CC',
      'en'
    );

    textTrack1.groupId = 'default-text-group';
    textTrack2.groupId = 'default-text-group';
    textTrack3.groupId = 'default-text-group';
    subtitleTrackController.groupId = 'default-text-group';

    textTrack1.mode = 'disabled';
    textTrack2.mode = 'disabled';
    textTrack3.mode = 'disabled';

    it('should set subtitleTrack if captions track is showing', function () {
      expect(subtitleTrackController.subtitleTrack).to.equal(-1);

      videoElement.textTracks[2].mode = 'showing';
      subtitleTrackController.onTextTracksChanged();

      expect(videoElement.textTracks[2].kind).to.equal('captions');
      expect(subtitleTrackController.subtitleTrack).to.equal(2);
    });

We should have another test that covers setting subtitleTrackController.subtitleTrack = 2; for this issue. That was a miss on my part. That seemed to be the core issue with #5292 (enabling tracks of kind "captions" via hls.subtitleTrack = was not doing anything. The filter is meant to ignore tracks of kind "metadata " (so ideally we should add one of those too in these tests).