Arnavion / libjass

Renders ASS subs in the browser.
Apache License 2.0
174 stars 29 forks source link

Font Rendering with @prefixed in front #84

Open Denoder opened 7 years ago

Denoder commented 7 years ago

There's a problem where fonts has @ in front of their names and doesnt get loaded

Example:

http://prntscr.com/dpx2ba

Arnavion commented 7 years ago

What exactly is the problem? Do you have an example page I can access?

Denoder commented 7 years ago

I temporarily fixed it by doing this:

            Object.defineProperty(Style.prototype, "fontName", {
                /**
                 * The name of this style's font.
                 *
                 * @type {string}
                 */
                get: function () {
                    return this._fontName.replace('@','');
                },
                enumerable: true,
                configurable: true
            });

Basically, when its getting the FontName from the .ass file there are fonts with @ infront of the font name (like @Arial) this would lead to naming the font-family css selector "with" the @ symbol in front of the name.

Arnavion commented 7 years ago

As far as I can tell every place that sets the font-family CSS property will quote the font name, so there shouldn't be a problem.

If your change makes it work, it sounds like even though the font file wants a font named @foo the actual font installed on your system is named foo. Is there some ASS feature that the @ in font names is supposed to be ignored?

Arnavion commented 7 years ago

Okay, I see there is some special meaning to leading @ https://github.com/libass/libass/blob/8b898d6b/libass/ass_parse.c#L107

Denoder commented 7 years ago

so do i just leave my change?

Arnavion commented 7 years ago

What you have will work. I think a better place to strip the leading @ will be in SpanStyles when setting the CSS property (in case there is some reason to keep the @ around for something else), but the end result will be the same for now.