Skip to content

Commit 2250324

Browse files
committed
fix: Fix enforcement of cue alignment styles
This uses flexbox once again to get proper positioning of cues. To compensate for the issues that originally made us remove flexbox, this adds a wrapper span inside the flexbox element. Summary of screenshot changes: - slight change to background sizing - ui-basic-cue - ui-cue-with-controls - ui-duplicate-cues - ui-end-time-edge-case - ui-flat-cue-bg - ui-two-basic-cues - background fills block with literal newline in text - ui-cue-with-newline - region anchored without padding - ui-region-position - new screenshots - *-nested-cues-with-linebreak - *-region-with-display-alignment (regression test for this issue) Closes shaka-project#3379 Change-Id: I8c678721d96662e0f8940cda12df4f5b5e5baf1e
1 parent de77787 commit 2250324

File tree

126 files changed

+93
-14
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

126 files changed

+93
-14
lines changed

lib/text/cue.js

+11
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,17 @@ shaka.text.Cue = class {
232232
};
233233
}
234234

235+
/**
236+
* @param {number} start
237+
* @param {number} end
238+
* @return {!shaka.text.Cue}
239+
*/
240+
static lineBreak(start, end) {
241+
const cue = new shaka.text.Cue(start, end, '');
242+
cue.lineBreak = true;
243+
return cue;
244+
}
245+
235246
/**
236247
* Create a copy of the cue with the same properties.
237248
* @return {!shaka.text.Cue}

lib/text/ui_text_displayer.js

+26-9
Original file line numberDiff line numberDiff line change
@@ -259,13 +259,22 @@ shaka.text.UITextDisplayer = class {
259259

260260
// Nested cues are inline elements. Top-level cues are block elements.
261261
const cueElement = shaka.util.Dom.createHTMLElement(type);
262-
263262
if (type != 'br') {
264263
this.setCaptionStyles_(cueElement, cue, isNested);
264+
}
265265

266-
for (const nestedCue of cue.nestedCues) {
267-
this.displayCue_(cueElement, nestedCue, /* isNested= */ true);
268-
}
266+
let wrapper = cueElement;
267+
if (!isNested && cue.nestedCues.length) {
268+
// Create a wrapper element which will serve to contain all children into
269+
// a single item. This ensures that nested span elements appear
270+
// horizontally and br elements occupy no vertical space.
271+
wrapper = shaka.util.Dom.createHTMLElement('span');
272+
wrapper.classList.add('shaka-text-wrapper');
273+
cueElement.appendChild(wrapper);
274+
}
275+
276+
for (const nestedCue of cue.nestedCues) {
277+
this.displayCue_(wrapper, nestedCue, /* isNested= */ true);
269278
}
270279

271280
container.appendChild(cueElement);
@@ -338,12 +347,20 @@ shaka.text.UITextDisplayer = class {
338347
// The displayAlign attribute specifies the vertical alignment of the
339348
// captions inside the text container. Before means at the top of the
340349
// text container, and after means at the bottom.
341-
if (cue.displayAlign == Cue.displayAlign.BEFORE) {
342-
style.verticalAlign = 'top';
343-
} else if (cue.displayAlign == Cue.displayAlign.CENTER) {
344-
style.verticalAlign = 'middle';
350+
if (isNested) {
351+
style.display = 'inline';
345352
} else {
346-
style.verticalAlign = 'bottom';
353+
style.display = 'flex';
354+
style.flexDirection = 'column';
355+
style.alignItems = 'center';
356+
357+
if (cue.displayAlign == Cue.displayAlign.BEFORE) {
358+
style.justifyContent = 'flex-start';
359+
} else if (cue.displayAlign == Cue.displayAlign.CENTER) {
360+
style.justifyContent = 'center';
361+
} else {
362+
style.justifyContent = 'flex-end';
363+
}
347364
}
348365

349366
if (!isLeaf) {

test/test/assets/screenshots/review.html

+2
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,15 @@
4545
'two-basic-cues',
4646
'duplicate-cues',
4747
'two-nested-cues',
48+
'nested-cues-with-linebreak',
4849
'region-position',
4950
'cue-with-controls',
5051
'bitmap-cue',
5152
'nested-cue-bg',
5253
'flat-cue-bg',
5354
'deeply-nested-cues',
5455
'end-time-edge-case',
56+
'region-with-display-alignment',
5557
];
5658

5759
const prefixes = [

test/text/ui_text_displayer_unit.js

+8-4
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ describe('UITextDisplayer', () => {
8888

8989
const textContainer =
9090
videoContainer.querySelector('.shaka-text-container');
91-
const captions = textContainer.querySelector('span');
91+
const captions =
92+
textContainer.querySelector('span:not(.shaka-text-wrapper)');
9293
const cssObj = parseCssText(captions.style.cssText);
9394

9495
const expectCssObj = {
@@ -140,7 +141,8 @@ describe('UITextDisplayer', () => {
140141
// Verify styles applied to the nested cues.
141142
const textContainer =
142143
videoContainer.querySelector('.shaka-text-container');
143-
const captions = textContainer.querySelector('span');
144+
const captions =
145+
textContainer.querySelector('span:not(.shaka-text-wrapper)');
144146
const cssObj = parseCssText(captions.style.cssText);
145147

146148
const expectCssObj = {
@@ -193,7 +195,8 @@ describe('UITextDisplayer', () => {
193195

194196
const textContainer =
195197
videoContainer.querySelector('.shaka-text-container');
196-
const captions = textContainer.querySelector('span');
198+
const captions =
199+
textContainer.querySelector('span:not(.shaka-text-wrapper)');
197200
const cssObj = parseCssText(captions.style.cssText);
198201
expect(cssObj).toEqual(
199202
jasmine.objectContaining({
@@ -223,7 +226,8 @@ describe('UITextDisplayer', () => {
223226

224227
const textContainer =
225228
videoContainer.querySelector('.shaka-text-container');
226-
const captions = textContainer.querySelector('span');
229+
const captions =
230+
textContainer.querySelector('span:not(.shaka-text-wrapper)');
227231
const cssObj = parseCssText(captions.style.cssText);
228232
expect(cssObj).toEqual(
229233
jasmine.objectContaining({'font-size': expectedFontSize}));

test/ui/text_displayer_layout_unit.js

+39
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,21 @@ filterDescribe('TextDisplayer layout', supportsScreenshots, () => {
299299
await checkScreenshot(prefix, 'two-nested-cues');
300300
});
301301

302+
// Distinct from "newline" test above, which has a literal \n character in
303+
// the text payload. This uses a nested "lineBreak" cue, which is what you
304+
// get with <br> in TTML.
305+
it('nested cues with linebreak', async () => {
306+
const cue = new shaka.text.Cue(0, 1, '');
307+
cue.nestedCues = [
308+
new shaka.text.Cue(0, 1, 'Captain\'s log,'),
309+
shaka.text.Cue.lineBreak(0, 1),
310+
new shaka.text.Cue(0, 1, 'stardate 41636.9'),
311+
];
312+
textDisplayer.append([cue]);
313+
314+
await checkScreenshot(prefix, 'nested-cues-with-linebreak');
315+
});
316+
302317
// Regression test for #2157 and #2584
303318
it('region positioning', async () => {
304319
const nestedCue = new shaka.text.Cue(
@@ -316,6 +331,30 @@ filterDescribe('TextDisplayer layout', supportsScreenshots, () => {
316331
await checkScreenshot(prefix, 'region-position');
317332
});
318333

334+
// Regression test for #3379, in which the displayAlign was not respected,
335+
// placing text at the top of the region instead of the bottom.
336+
it('region with display alignment', async () => {
337+
const cue = new shaka.text.Cue(0, 1, '');
338+
339+
cue.region.id = '1';
340+
cue.region.viewportAnchorX = 10;
341+
cue.region.viewportAnchorY = 10;
342+
cue.region.width = 80;
343+
cue.region.height = 80;
344+
345+
cue.positionAlign = shaka.text.Cue.positionAlign.CENTER;
346+
cue.lineAlign = shaka.text.Cue.lineAlign.CENTER;
347+
cue.displayAlign = shaka.text.Cue.displayAlign.AFTER;
348+
349+
cue.nestedCues = [
350+
// For those who don't speak Unicode, \xbf is an upside down "?".
351+
new shaka.text.Cue(0, 1, '\xbfBien?'),
352+
];
353+
textDisplayer.append([cue]);
354+
355+
await checkScreenshot(prefix, 'region-with-display-alignment');
356+
});
357+
319358
// Regression test for #2188
320359
it('bitmap-based cues', async () => {
321360
const cue = new shaka.text.Cue(0, 1, '');

ui/less/containers.less

+7-1
Original file line numberDiff line numberDiff line change
@@ -226,12 +226,18 @@
226226
transition-delay: 500ms;
227227

228228
/* These are defaults which are overridden by JS or cue styles. */
229-
span {
229+
span:not(.shaka-text-wrapper) {
230+
display: inline;
230231
font-size: 20px;
231232
line-height: 1.4; // relative to font size.
232233
background-color: rgba(0, 0, 0, 0.8);
233234
color: rgb(255, 255, 255);
234235
}
236+
237+
span.shaka-text-wrapper {
238+
display: inline;
239+
background: none;
240+
}
235241
}
236242

237243
.shaka-controls-container[shown="true"] ~ .shaka-text-container {

0 commit comments

Comments
 (0)