Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(FEC-11606): Add TextTrack label to TIMED_METADATA event payload #609

Merged
merged 1 commit into from
Oct 4, 2021

Conversation

semarche-kaltura
Copy link
Contributor

Description of the Changes

TIMED_METADATA event payload contains label to define which TextTrack dispatched event

CheckLists

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • test are passing in local environment
  • Travis tests are passing (or test results are not worse than on master branch :))
  • Docs have been updated

@semarche-kaltura semarche-kaltura changed the title FEC-11606: Expose TextTrack label on TIMED_METADATA event fix(FEC-11606): Add TextTrack label to TIMED_METADATA event payload Oct 4, 2021
@semarche-kaltura semarche-kaltura merged commit 9465501 into master Oct 4, 2021
@semarche-kaltura semarche-kaltura deleted the FEC-11606-text-track-label branch October 4, 2021 13:16
@@ -1165,7 +1165,7 @@ export default class Html5 extends FakeEventTarget implements IEngine {
const listenToCueChange = track => {
track.mode = PKTextTrack.MODE.HIDDEN;
this._eventManager.listen(track, 'cuechange', () => {
this.dispatchEvent(new FakeEvent(CustomEventType.TIMED_METADATA, {cues: Array.from(track.activeCues)}));
this.dispatchEvent(new FakeEvent(CustomEventType.TIMED_METADATA, {cues: Array.from(track.activeCues), label: track.label}));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Label is the data source attribute of the text track but who said times metadata data source will always be a text track.
This is an example of leaking implementation detail.
I assume this was done for "easier" filtering but it leads to tightly coupling text track and the timed metadata event and needs to be removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants