Fix for TTML alignment (non-centered) in Firefox #588
Conversation
… defaults to centered position which causes problem when TTML alignment is left-adjusted for example
|
Testing in progress... |
|
All tests passed! |
|
@ismena, this looks reasonable to me, but you have more experience here. Any objections? |
| @@ -332,12 +343,15 @@ shaka.media.TtmlTextParser.addStyle_ = function( | |||
| cueElement, region, styles, 'tts:textAlign'); | |||
| if (align) { | |||
| cue.align = align; | |||
| if (align == 'center' && cue.align != 'center') { | |||
| // Workaround for a Chrome bug http://crbug.com/663797 | |||
| // Chrome does not support align = 'center' | |||
There was a problem hiding this comment.
I'm a little lost here. Did something change in terms of logic on lines 335-351?
I suspect you might be working off an older version of the file (we refactored it a bit since your last commit). If that's the case, let's stick to the newer version.
Looks good otherwise.
There was a problem hiding this comment.
This was actually an intentional change as setting the cue.position to auto when textAlign is center is not because of a bug in Chrome but rather due to the differences between TTML and VTT. It should not impose any logical change from previous commit but make it more clear that it is not related to the Chrome bug so it will risk to be removed when that cue.align bug is fixed.
There was a problem hiding this comment.
Oh, I see. In that case we are good to go.
This commit fixes non-centered TTML alignment issues in Firefox which defaults to centered position which causes problem when TTML alignment is left-adjusted for example.
github-actions
bot
added
the
status: archived
It was discovered that TTML alignments that is not centered (e.g. left) was shown incorrectly in Firefox. The reason I discovered was that the cue.positionAlign was not set and defaulted to "center". This caused the subtitles to be positioned on the left (when tts:textAlign=left) but with the text center aligned.
Note that for Firefox v48 or lower the implementation of cue.positionAlign did not fully follow the spec and was corrected in v49 and above.