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

Text Track Cue Property "line" doesn't work with number #4160

Closed
5 tasks done
dylan17th opened this issue Mar 28, 2023 · 2 comments
Closed
5 tasks done

Text Track Cue Property "line" doesn't work with number #4160

dylan17th opened this issue Mar 28, 2023 · 2 comments
Assignees
Labels
Milestone

Comments

@dylan17th
Copy link

dylan17th commented Mar 28, 2023

Environment
  • Link to playable MPD file:
  • Dash.js version: 4.4.1
  • Browser name/version: ALL
  • OS name/version: Monterey version 12.0.1
Steps to reproduce
  1. Use any valid mpd file with a vtt text track
  2. Set the property to -3 on any cue in the vtt track (honestly can be any valid number)
  3. Play video
Observed behavior

Depending on setup it either doesn't show or throws an error.

Console output
[TextTracks] TypeError: Failed to set the 'line' property on 'VTTCue': The provided value '-3' is not a valid enum value of type AutoKeyword.
Expected behavior

When any valid number or 'auto' string is passed to the line property, it should add it to the cues line property correctly.

Current Implementation

The array passed in will always be an array of strings and unless the value passed in for a line is a percentage, it won't be parse as a number. Essentially, a user can't pass a valid number value for the line property and has to pass a percentage as a number in order for it to work. I understand why this is in place for the position and size properties, but I'm confused why its being done for the line property and according to MDN, a percent isn't a valid value for the line attribute. However, I know if you make snapToLine false, it will use the provided value as a percentage, but as far as I can tell, I don't see where you are allowing a consumer to do that.

    function getCaptionStyles(arr) {
        const styleObject = {};
        arr.forEach(function (element) {
            if (element.split(/:/).length > 1) {
                let val = element.split(/:/)[1];
                if (val && val.search(/%/) != -1) {
                    val = parseInt(val.replace(/%/, ''), 10);
                }
                if (element.match(/align/) || element.match(/A/)) {
                    styleObject.align = val;
                }
                if (element.match(/line/) || element.match(/L/) ) {
                    styleObject.line = val;
                }
                if (element.match(/position/) || element.match(/P/) ) {
                    styleObject.position = val;
                }
                if (element.match(/size/) || element.match(/S/)) {
                    styleObject.size = val;
                }
            }
        });

        return styleObject;
    }

Suggested Implementation

This implementation will allow users to pass all valid values to the line attribute and only throw errors when an invalid value is provided like any percentage.

    function getCaptionStyles(arr) {
        const styleObject = {};

        function convertPercentValueToNumber(val) {
            return parseInt(val.replace(/%/, ''), 10);
        }

        arr.forEach(function (element) {
            if (element.split(/:/).length > 1) {
                let val = element.split(/:/)[1];

                if (element.match(/align/) || element.match(/A/)) {
                    styleObject.align = val;
                }
                if (element.match(/line/) || element.match(/L/)) {
                    styleObject.line = +val || val;
                }
                if (element.match(/position/) || element.match(/P/)) {
                    styleObject.position = convertPercentValueToNumber(val);
                }
                if (element.match(/size/) || element.match(/S/)) {
                    styleObject.size = convertPercentValueToNumber(val);
                }
            }
        });

        return styleObject;
    }
@dylan17th dylan17th added the Bug label Mar 28, 2023
@dsilhavy dsilhavy added this to the 4.7.1 milestone Apr 25, 2023
@dsilhavy dsilhavy self-assigned this Jun 6, 2023
@dsilhavy
Copy link
Collaborator

dsilhavy commented Jun 6, 2023

This was fixed in #4167 , @dylan17th can you confirm that this is working as expected?

@dsilhavy
Copy link
Collaborator

Closing fixed in #4167

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

No branches or pull requests

2 participants