Skip to content

Commit e1cebda

Browse files
valotvincevalotvince
committed
review comments
1 parent 35345f2 commit e1cebda

File tree

2 files changed

+47
-43
lines changed

2 files changed

+47
-43
lines changed

‎lib/text/ttml_text_parser.js‎

Lines changed: 41 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ shaka.text.TtmlTextParser = class {
175175

176176
/**
177177
* Get the leaf nodes that can act as cues
178-
* (begin and end attributes)
178+
* (at least begin attribute)
179179
*
180180
* @param {Element} element
181181
* @return {!NodeList.<!Element>|Array}
@@ -186,7 +186,7 @@ shaka.text.TtmlTextParser = class {
186186
return [];
187187
}
188188

189-
return element.querySelectorAll('[begin][end]');
189+
return element.querySelectorAll('[begin]');
190190
}
191191

192192

@@ -218,6 +218,8 @@ shaka.text.TtmlTextParser = class {
218218
trimmed = trimmed.replace(/\s+/g, ' ');
219219

220220
payload += trimmed;
221+
} else {
222+
payload += childNode.textContent;
221223
}
222224
}
223225

@@ -235,14 +237,14 @@ shaka.text.TtmlTextParser = class {
235237
* @param {!Array.<!Element>} regionElements
236238
* @param {!Array.<!shaka.text.CueRegion>} cueRegions
237239
* @param {boolean} whitespaceTrim
238-
* @param {boolean} isChild
240+
* @param {boolean} isNested
239241
* @return {shaka.text.Cue}
240242
* @private
241243
*/
242244
static parseCue_(
243245
cueElement, offset, rateInfo, metadataElements, styles, regionElements,
244-
cueRegions, whitespaceTrim, isChild) {
245-
if (isChild && cueElement.nodeName === 'br') {
246+
cueRegions, whitespaceTrim, isNested) {
247+
if (isNested && cueElement.nodeName == 'br') {
246248
const cue = new shaka.text.Cue(0, 0, '');
247249
cue.spacer = true;
248250

@@ -256,7 +258,7 @@ shaka.text.TtmlTextParser = class {
256258
if (
257259
/^[\s\n]*$/.test(cueElement.textContent) ||
258260
(
259-
!isChild &&
261+
!isNested &&
260262
!cueElement.hasAttribute('begin') &&
261263
!cueElement.hasAttribute('end')
262264
)
@@ -271,6 +273,21 @@ shaka.text.TtmlTextParser = class {
271273
cueElement.getAttribute('end'), rateInfo);
272274
const duration = shaka.text.TtmlTextParser.parseTime_(
273275
cueElement.getAttribute('dur'), rateInfo);
276+
277+
if (end == null && duration != null) {
278+
end = start + duration;
279+
}
280+
281+
if (!isNested && (start == null || end == null)) {
282+
throw new shaka.util.Error(
283+
shaka.util.Error.Severity.CRITICAL,
284+
shaka.util.Error.Category.TEXT,
285+
shaka.util.Error.Code.INVALID_TEXT_CUE);
286+
}
287+
288+
start += offset;
289+
end += offset;
290+
274291
let payload = '';
275292
const nestedCues = [];
276293
// If one of the children is text node type
@@ -286,9 +303,8 @@ shaka.text.TtmlTextParser = class {
286303
whitespaceTrim,
287304
);
288305
} else {
289-
for (let i = 0; i < cueElement.childNodes.length; i++) {
290-
const childNode = cueElement.childNodes[i];
291-
const childCue = shaka.text.TtmlTextParser.parseCue_(
306+
for (const childNode of cueElement.childNodes) {
307+
const nestedCue = shaka.text.TtmlTextParser.parseCue_(
292308
/** @type {!Element} */ (childNode),
293309
offset,
294310
rateInfo,
@@ -297,40 +313,26 @@ shaka.text.TtmlTextParser = class {
297313
regionElements,
298314
cueRegions,
299315
whitespaceTrim,
300-
true
316+
/* isNested */ true
301317
);
302318

303-
if (childCue) {
304-
nestedCues.push(childCue);
319+
if (nestedCue) {
320+
nestedCues.push(nestedCue);
305321
}
306322
}
307323
}
308324

309-
if (end == null && duration != null) {
310-
end = start + duration;
311-
}
312-
313-
if (!isChild && (start == null || end == null)) {
314-
throw new shaka.util.Error(
315-
shaka.util.Error.Severity.CRITICAL,
316-
shaka.util.Error.Category.TEXT,
317-
shaka.util.Error.Code.INVALID_TEXT_CUE);
318-
}
319-
320-
start += offset;
321-
end += offset;
322-
323325
const cue = new shaka.text.Cue(start, end, payload);
324326
cue.nestedCues = nestedCues;
325327

326328
// Get other properties if available.
327-
const regionElement = shaka.text.TtmlTextParser.getElementFromCollection_(
329+
const regionElement = shaka.text.TtmlTextParser.getElementsFromCollection_(
328330
cueElement, 'region', regionElements, /* prefix= */ '')[0];
329331
if (regionElement && regionElement.getAttribute('xml:id')) {
330332
const regionId = regionElement.getAttribute('xml:id');
331333
cue.region = cueRegions.filter((region) => region.id == regionId)[0];
332334
}
333-
const imageElement = shaka.text.TtmlTextParser.getElementFromCollection_(
335+
const imageElement = shaka.text.TtmlTextParser.getElementsFromCollection_(
334336
cueElement, 'smpte:backgroundImage', metadataElements, '#')[0];
335337
shaka.text.TtmlTextParser.addStyle_(
336338
cue,
@@ -660,7 +662,7 @@ shaka.text.TtmlTextParser = class {
660662
}
661663
}
662664

663-
const style = shaka.text.TtmlTextParser.getElementFromCollection_(
665+
const style = shaka.text.TtmlTextParser.getElementsFromCollection_(
664666
region, 'style', styles, /* prefix= */ '')[0];
665667

666668
if (style) {
@@ -696,8 +698,10 @@ shaka.text.TtmlTextParser = class {
696698
return elementAttribute;
697699
}
698700

699-
const inheritedStyles = shaka.text.TtmlTextParser.getElementFromCollection_(
700-
cueElement, 'style', styles, /* prefix= */ '');
701+
const inheritedStyles =
702+
shaka.text.TtmlTextParser.getElementsFromCollection_(
703+
cueElement, 'style', styles, /* prefix= */ ''
704+
);
701705

702706
let styleValue = null;
703707

@@ -719,7 +723,7 @@ shaka.text.TtmlTextParser = class {
719723

720724

721725
/**
722-
* Selects an item from |collection| whose id matches |attributeName|
726+
* Selects items from |collection| whose id matches |attributeName|
723727
* from |element|.
724728
*
725729
* @param {Element} element
@@ -729,7 +733,7 @@ shaka.text.TtmlTextParser = class {
729733
* @return {Array.<!Element>}
730734
* @private
731735
*/
732-
static getElementFromCollection_(
736+
static getElementsFromCollection_(
733737
element, attributeName, collection, prefixName) {
734738
const items = [];
735739

@@ -745,12 +749,12 @@ shaka.text.TtmlTextParser = class {
745749
// <span style="style1 style2">A cue</span>
746750
const itemNames = attributeValue.split(' ');
747751

748-
for (let i = 0; i < itemNames.length; i++) {
749-
for (let k = 0; k < collection.length; k++) {
752+
for (const name of itemNames) {
753+
for (const item of collection) {
750754
if (
751-
(prefixName + collection[k].getAttribute('xml:id')) == itemNames[i]
755+
(prefixName + item.getAttribute('xml:id')) == name
752756
) {
753-
items.push(collection[k]);
757+
items.push(item);
754758
break;
755759
}
756760
}

‎test/text/ttml_text_parser_unit.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ describe('TtmlTextParser', () => {
5959
{
6060
startTime: 62.03,
6161
endTime: 62.05,
62-
nestedCues: [{payload: '\n A B C \n '}],
62+
nestedCues: [{payload: ' A B C '}],
6363
payload: '',
6464
},
6565
],
@@ -89,22 +89,22 @@ describe('TtmlTextParser', () => {
8989

9090
it('rejects invalid time format', () => {
9191
errorHelper(shaka.util.Error.Code.INVALID_TEXT_CUE,
92-
'<tt><body><p begin="test" end="test"></p></body></tt>');
92+
'<tt><body><p begin="test" end="test">My very own cue</p></body></tt>');
9393
errorHelper(shaka.util.Error.Code.INVALID_TEXT_CUE,
94-
'<tt><body><p begin="3.45" end="1a"></p></body></tt>');
94+
'<tt><body><p begin="3.45" end="1a">An invalid cue</p></body></tt>');
9595
});
9696

9797
it('supports spans as nestedCues of paragraphs', () => {
9898
verifyHelper(
9999
[
100100
{
101-
start: 62.05,
102-
end: 3723.2,
101+
startTime: 62.05,
102+
endTime: 3723.2,
103103
payload: '',
104104
nestedCues: [
105105
{payload: 'First cue'},
106106
{payload: '', spacer: true},
107-
{payload: 'Second cuee'},
107+
{payload: 'Second cue'},
108108
],
109109
},
110110
],

0 commit comments

Comments
 (0)