From c0fbfbf52ea4d439c5184bdf4616f0d483096602 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Thu, 22 Jul 2021 16:52:56 -0400 Subject: [PATCH 1/8] fix: remove uses of innerHTML to prevent xss --- src/js/button.js | 4 ++- src/js/clickable-component.js | 6 +++- .../audio-track-menu-item.js | 29 ++++++++++------- src/js/control-bar/live-display.js | 11 ++++++- .../playback-rate-menu-button.js | 4 +-- src/js/control-bar/seek-to-live.js | 2 +- .../spacer-controls/custom-control-spacer.js | 12 +++---- .../subs-caps-menu-item.js | 31 ++++++++++++------- .../control-bar/time-controls/time-display.js | 10 ++++-- .../control-bar/time-controls/time-divider.js | 13 ++++++-- .../volume-control/volume-level.js | 12 +++++-- src/js/loading-spinner.js | 2 +- src/js/menu/menu-button.js | 2 +- src/js/menu/menu-item.js | 12 +++++-- 14 files changed, 102 insertions(+), 48 deletions(-) diff --git a/src/js/button.js b/src/js/button.js index bbfd2f14d5..6e74af09f2 100644 --- a/src/js/button.js +++ b/src/js/button.js @@ -34,7 +34,6 @@ class Button extends ClickableComponent { tag = 'button'; props = assign({ - innerHTML: '', className: this.buildCSSClass() }, props); @@ -46,6 +45,9 @@ class Button extends ClickableComponent { }, attributes); const el = Component.prototype.createEl.call(this, tag, props, attributes); + const span = Component.prototype.createEl.call(this, 'span', {className: 'vjs-icon-placeholder'}); + + el.appendChild(span); this.createControlTextEl(el); diff --git a/src/js/clickable-component.js b/src/js/clickable-component.js index fbdecce10a..37d74eb938 100644 --- a/src/js/clickable-component.js +++ b/src/js/clickable-component.js @@ -57,7 +57,6 @@ class ClickableComponent extends Component { */ createEl(tag = 'div', props = {}, attributes = {}) { props = assign({ - innerHTML: '', className: this.buildCSSClass(), tabIndex: 0 }, props); @@ -74,6 +73,11 @@ class ClickableComponent extends Component { this.tabIndex_ = props.tabIndex; const el = super.createEl(tag, props, attributes); + const span = super.createEl('span', { + className: 'vjs-icon-placeholder' + }); + + el.appendChild(span); this.createControlTextEl(el); diff --git a/src/js/control-bar/audio-track-controls/audio-track-menu-item.js b/src/js/control-bar/audio-track-controls/audio-track-menu-item.js index 86dfe0f010..980dc4da4b 100644 --- a/src/js/control-bar/audio-track-controls/audio-track-menu-item.js +++ b/src/js/control-bar/audio-track-controls/audio-track-menu-item.js @@ -3,7 +3,6 @@ */ import MenuItem from '../../menu/menu-item.js'; import Component from '../../component.js'; -import {assign} from '../../utils/obj'; /** * An {@link AudioTrack} {@link MenuItem} @@ -46,20 +45,28 @@ class AudioTrackMenuItem extends MenuItem { } createEl(type, props, attrs) { - let innerHTML = `${this.localize(this.options_.label)}`; + const el = super.createEl(type, props, attrs); + const parentSpan = super.createEl('span', { + className: 'vjs-menu-item-text', + textContent: this.localize(this.options_.label) + }); if (this.options_.track.kind === 'main-desc') { - innerHTML += ` - - ${this.localize('Descriptions')} - `; + const icon = super.createEl('span', { + className: 'vjs-icon-placeholder' + }, { + 'aria-hidden': true + }); + const controlText = super.createEl('span', { + className: 'vjs-control-text', + textContent: this.localize('Descriptions') + }); + + parentSpan.appendChild(icon); + parentSpan.appendChild(controlText); } - innerHTML += ''; - - const el = super.createEl(type, assign({ - innerHTML - }, props), attrs); + el.appendChild(parentSpan); return el; } diff --git a/src/js/control-bar/live-display.js b/src/js/control-bar/live-display.js index f3c0beddca..850454a0fb 100644 --- a/src/js/control-bar/live-display.js +++ b/src/js/control-bar/live-display.js @@ -3,6 +3,7 @@ */ import Component from '../component'; import * as Dom from '../utils/dom.js'; +import document from 'global/document'; // TODO - Future make it click to snap to live @@ -40,13 +41,21 @@ class LiveDisplay extends Component { className: 'vjs-live-control vjs-control' }); + const textNode = document.createTextNode(this.localize('LIVE')); + this.contentEl_ = Dom.createEl('div', { className: 'vjs-live-display', - innerHTML: `${this.localize('Stream Type')}\u00a0${this.localize('LIVE')}` + textContent: `${this.localize('Stream Type')}\u00a0` }, { 'aria-live': 'off' }); + const span = Dom.createEl('span', { + className: 'vjs-control-text' + }); + + this.contentEl_.appendChild(span); + this.contentEl_.appendChild(textNode); el.appendChild(this.contentEl_); return el; } diff --git a/src/js/control-bar/playback-rate-menu/playback-rate-menu-button.js b/src/js/control-bar/playback-rate-menu/playback-rate-menu-button.js index 17dc275163..d9c5d585ff 100644 --- a/src/js/control-bar/playback-rate-menu/playback-rate-menu-button.js +++ b/src/js/control-bar/playback-rate-menu/playback-rate-menu-button.js @@ -49,7 +49,7 @@ class PlaybackRateMenuButton extends MenuButton { this.labelEl_ = Dom.createEl('div', { className: 'vjs-playback-rate-value', id: this.labelElId_, - innerHTML: '1x' + textContent: '1x' }); el.appendChild(this.labelEl_); @@ -190,7 +190,7 @@ class PlaybackRateMenuButton extends MenuButton { */ updateLabel(event) { if (this.playbackRateSupported()) { - this.labelEl_.innerHTML = this.player().playbackRate() + 'x'; + this.labelEl_.textContent = this.player().playbackRate() + 'x'; } } diff --git a/src/js/control-bar/seek-to-live.js b/src/js/control-bar/seek-to-live.js index 82c3fc82c1..9d896f03d0 100644 --- a/src/js/control-bar/seek-to-live.js +++ b/src/js/control-bar/seek-to-live.js @@ -45,7 +45,7 @@ class SeekToLive extends Button { this.textEl_ = Dom.createEl('span', { className: 'vjs-seek-to-live-text', - innerHTML: this.localize('LIVE') + textContent: this.localize('LIVE') }, { 'aria-hidden': 'true' }); diff --git a/src/js/control-bar/spacer-controls/custom-control-spacer.js b/src/js/control-bar/spacer-controls/custom-control-spacer.js index bc14447953..f627c0c039 100644 --- a/src/js/control-bar/spacer-controls/custom-control-spacer.js +++ b/src/js/control-bar/spacer-controls/custom-control-spacer.js @@ -28,14 +28,12 @@ class CustomControlSpacer extends Spacer { * The element that was created. */ createEl() { - const el = super.createEl({ - className: this.buildCSSClass() + return super.createEl({ + className: this.buildCSSClass(), + // No-flex/table-cell mode requires there be some content + // in the cell to fill the remaining space of the table. + textContent: '\u00a0' }); - - // No-flex/table-cell mode requires there be some content - // in the cell to fill the remaining space of the table. - el.innerHTML = '\u00a0'; - return el; } } diff --git a/src/js/control-bar/text-track-controls/subs-caps-menu-item.js b/src/js/control-bar/text-track-controls/subs-caps-menu-item.js index 403b9867d8..86d1c770d3 100644 --- a/src/js/control-bar/text-track-controls/subs-caps-menu-item.js +++ b/src/js/control-bar/text-track-controls/subs-caps-menu-item.js @@ -3,7 +3,6 @@ */ import TextTrackMenuItem from './text-track-menu-item.js'; import Component from '../../component.js'; -import {assign} from '../../utils/obj'; /** * SubsCapsMenuItem has an [cc] icon to distinguish captions from subtitles @@ -14,20 +13,28 @@ import {assign} from '../../utils/obj'; class SubsCapsMenuItem extends TextTrackMenuItem { createEl(type, props, attrs) { - let innerHTML = `${this.localize(this.options_.label)}`; + const el = super.createEl(type, props, attrs); + const parentSpan = super.createEl('span', { + className: 'vjs-menu-item-text', + textContent: this.localize(this.options_.label) + }); - if (this.options_.track.kind === 'captions') { - innerHTML += ` - - ${this.localize('Captions')} - `; - } + if (this.options_.track.kind === 'main-desc') { + const icon = super.createEl('span', { + className: 'vjs-icon-placeholder' + }, { + 'aria-hidden': true + }); + const controlText = super.createEl('span', { + className: 'vjs-control-text', + textContent: this.localize('captions') + }); - innerHTML += ''; + parentSpan.appendChild(icon); + parentSpan.appendChild(controlText); + } - const el = super.createEl(type, assign({ - innerHTML - }, props), attrs); + el.appendChild(parentSpan); return el; } diff --git a/src/js/control-bar/time-controls/time-display.js b/src/js/control-bar/time-controls/time-display.js index 7fbea15615..68a3e2daf3 100644 --- a/src/js/control-bar/time-controls/time-display.js +++ b/src/js/control-bar/time-controls/time-display.js @@ -39,9 +39,15 @@ class TimeDisplay extends Component { createEl() { const className = this.buildCSSClass(); const el = super.createEl('div', { - className: `${className} vjs-time-control vjs-control`, - innerHTML: `${this.localize(this.labelText_)}\u00a0` + className: `${className} vjs-time-control vjs-control` }); + const span = Dom.createEl('span', { + className: 'vjs-control-text', + role: 'presentation', + textContent: `${this.localize(this.labelText_)}\u00a0` + }); + + el.appendChild(span); this.contentEl_ = Dom.createEl('span', { className: `${className}-display` diff --git a/src/js/control-bar/time-controls/time-divider.js b/src/js/control-bar/time-controls/time-divider.js index d30e09a546..fbdaad9036 100644 --- a/src/js/control-bar/time-controls/time-divider.js +++ b/src/js/control-bar/time-controls/time-divider.js @@ -18,15 +18,22 @@ class TimeDivider extends Component { * The element that was created. */ createEl() { - return super.createEl('div', { - className: 'vjs-time-control vjs-time-divider', - innerHTML: '
/
' + const el = super.createEl('div', { + className: 'vjs-time-control vjs-time-divider' }, { // this element and its contents can be hidden from assistive techs since // it is made extraneous by the announcement of the control text // for the current time and duration displays 'aria-hidden': true }); + + const div = super.createEl('div'); + const span = super.createEl('span'); + + div.appendChild(span); + el.appendChild(div); + + return el; } } diff --git a/src/js/control-bar/volume-control/volume-level.js b/src/js/control-bar/volume-control/volume-level.js index 88bb0fe448..3b874c65eb 100644 --- a/src/js/control-bar/volume-control/volume-level.js +++ b/src/js/control-bar/volume-control/volume-level.js @@ -17,10 +17,16 @@ class VolumeLevel extends Component { * The element that was created. */ createEl() { - return super.createEl('div', { - className: 'vjs-volume-level', - innerHTML: '' + const el = super.createEl('div', { + className: 'vjs-volume-level' }); + const span = super.createEl('span', { + className: 'vjs-control-text' + }); + + el.appendChild(span); + + return el; } } diff --git a/src/js/loading-spinner.js b/src/js/loading-spinner.js index 93b6e49600..02e5f952f5 100644 --- a/src/js/loading-spinner.js +++ b/src/js/loading-spinner.js @@ -22,7 +22,7 @@ class LoadingSpinner extends Component { const playerType = this.localize(isAudio ? 'Audio Player' : 'Video Player'); const controlText = dom.createEl('span', { className: 'vjs-control-text', - innerHTML: this.localize('{1} is loading.', [playerType]) + textContent: this.localize('{1} is loading.', [playerType]) }); const el = super.createEl('div', { diff --git a/src/js/menu/menu-button.js b/src/js/menu/menu-button.js index c341d45f72..6b9c48f9d7 100644 --- a/src/js/menu/menu-button.js +++ b/src/js/menu/menu-button.js @@ -116,7 +116,7 @@ class MenuButton extends Component { if (this.options_.title) { const titleEl = Dom.createEl('li', { className: 'vjs-menu-title', - innerHTML: toTitleCase(this.options_.title), + textContent: toTitleCase(this.options_.title), tabIndex: -1 }); diff --git a/src/js/menu/menu-item.js b/src/js/menu/menu-item.js index c34622f103..725a88e567 100644 --- a/src/js/menu/menu-item.js +++ b/src/js/menu/menu-item.js @@ -63,11 +63,19 @@ class MenuItem extends ClickableComponent { // The control is textual, not just an icon this.nonIconControl = true; - return super.createEl('li', assign({ + const el = super.createEl('li', assign({ className: 'vjs-menu-item', - innerHTML: `${this.localize(this.options_.label)}`, tabIndex: -1 }, props), attrs); + + const span = super.createEl('span', { + className: 'vjs-menu-item-text', + textContent: this.localize(this.options_.label) + }); + + el.appendChild(span); + + return el; } /** From 8cebce940b3cb0fcd6890835c6df55ed30e2380a Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Thu, 22 Jul 2021 17:06:29 -0400 Subject: [PATCH 2/8] slight refactor and fixes looking at diff --- src/js/button.js | 10 ++++++--- src/js/clickable-component.js | 11 +++++----- .../audio-track-menu-item.js | 17 +++++++-------- src/js/control-bar/live-display.js | 9 +++----- .../subs-caps-menu-item.js | 21 +++++++++---------- .../volume-control/volume-level.js | 7 +++---- src/js/menu/menu-item.js | 6 ++---- 7 files changed, 39 insertions(+), 42 deletions(-) diff --git a/src/js/button.js b/src/js/button.js index 6e74af09f2..010f9de647 100644 --- a/src/js/button.js +++ b/src/js/button.js @@ -6,6 +6,7 @@ import Component from './component'; import log from './utils/log.js'; import {assign} from './utils/obj'; import keycode from 'keycode'; +import {createEl} from './utils/dom.js'; /** * Base class for all buttons. @@ -44,10 +45,13 @@ class Button extends ClickableComponent { type: 'button' }, attributes); - const el = Component.prototype.createEl.call(this, tag, props, attributes); - const span = Component.prototype.createEl.call(this, 'span', {className: 'vjs-icon-placeholder'}); + const el = createEl(tag, props, attributes); - el.appendChild(span); + el.appendChild(createEl('span', { + className: 'vjs-icon-placeholder' + }, { + 'aria-hidden': true + })); this.createControlTextEl(el); diff --git a/src/js/clickable-component.js b/src/js/clickable-component.js index 37d74eb938..f9d5021bad 100644 --- a/src/js/clickable-component.js +++ b/src/js/clickable-component.js @@ -72,12 +72,13 @@ class ClickableComponent extends Component { this.tabIndex_ = props.tabIndex; - const el = super.createEl(tag, props, attributes); - const span = super.createEl('span', { - className: 'vjs-icon-placeholder' - }); + const el = Dom.createEl(tag, props, attributes); - el.appendChild(span); + el.appendChild(Dom.createEl('span', { + className: 'vjs-icon-placeholder' + }, { + 'aria-hidden': true + })); this.createControlTextEl(el); diff --git a/src/js/control-bar/audio-track-controls/audio-track-menu-item.js b/src/js/control-bar/audio-track-controls/audio-track-menu-item.js index 980dc4da4b..7ad18a5833 100644 --- a/src/js/control-bar/audio-track-controls/audio-track-menu-item.js +++ b/src/js/control-bar/audio-track-controls/audio-track-menu-item.js @@ -3,6 +3,7 @@ */ import MenuItem from '../../menu/menu-item.js'; import Component from '../../component.js'; +import document from 'global/document'; /** * An {@link AudioTrack} {@link MenuItem} @@ -47,23 +48,21 @@ class AudioTrackMenuItem extends MenuItem { createEl(type, props, attrs) { const el = super.createEl(type, props, attrs); const parentSpan = super.createEl('span', { - className: 'vjs-menu-item-text', - textContent: this.localize(this.options_.label) + className: 'vjs-menu-item-text' }); + parentSpan.appendChild(document.createTextNode(this.localize(this.options_.label))); + if (this.options_.track.kind === 'main-desc') { - const icon = super.createEl('span', { + parentSpan.appendChild(super.createEl('span', { className: 'vjs-icon-placeholder' }, { 'aria-hidden': true - }); - const controlText = super.createEl('span', { + })); + parentSpan.appendChild(super.createEl('span', { className: 'vjs-control-text', textContent: this.localize('Descriptions') - }); - - parentSpan.appendChild(icon); - parentSpan.appendChild(controlText); + })); } el.appendChild(parentSpan); diff --git a/src/js/control-bar/live-display.js b/src/js/control-bar/live-display.js index 850454a0fb..21e1d756a7 100644 --- a/src/js/control-bar/live-display.js +++ b/src/js/control-bar/live-display.js @@ -41,8 +41,6 @@ class LiveDisplay extends Component { className: 'vjs-live-control vjs-control' }); - const textNode = document.createTextNode(this.localize('LIVE')); - this.contentEl_ = Dom.createEl('div', { className: 'vjs-live-display', textContent: `${this.localize('Stream Type')}\u00a0` @@ -50,12 +48,11 @@ class LiveDisplay extends Component { 'aria-live': 'off' }); - const span = Dom.createEl('span', { + this.contentEl_.appendChild(Dom.createEl('span', { className: 'vjs-control-text' - }); + })); + this.contentEl_.appendChild(document.createTextNode(this.localize('LIVE'))); - this.contentEl_.appendChild(span); - this.contentEl_.appendChild(textNode); el.appendChild(this.contentEl_); return el; } diff --git a/src/js/control-bar/text-track-controls/subs-caps-menu-item.js b/src/js/control-bar/text-track-controls/subs-caps-menu-item.js index 86d1c770d3..ee10a4fc7c 100644 --- a/src/js/control-bar/text-track-controls/subs-caps-menu-item.js +++ b/src/js/control-bar/text-track-controls/subs-caps-menu-item.js @@ -3,6 +3,7 @@ */ import TextTrackMenuItem from './text-track-menu-item.js'; import Component from '../../component.js'; +import document from 'global/document'; /** * SubsCapsMenuItem has an [cc] icon to distinguish captions from subtitles @@ -15,23 +16,21 @@ class SubsCapsMenuItem extends TextTrackMenuItem { createEl(type, props, attrs) { const el = super.createEl(type, props, attrs); const parentSpan = super.createEl('span', { - className: 'vjs-menu-item-text', - textContent: this.localize(this.options_.label) + className: 'vjs-menu-item-text' }); - if (this.options_.track.kind === 'main-desc') { - const icon = super.createEl('span', { + parentSpan.appendChild(document.createTextNode(this.localize(this.options_.label))); + + if (this.options_.track.kind === 'captions') { + parentSpan.appendChild(super.createEl('span', { className: 'vjs-icon-placeholder' }, { 'aria-hidden': true - }); - const controlText = super.createEl('span', { + })); + parentSpan.appendChild(super.createEl('span', { className: 'vjs-control-text', - textContent: this.localize('captions') - }); - - parentSpan.appendChild(icon); - parentSpan.appendChild(controlText); + textContent: this.localize('Captions') + })); } el.appendChild(parentSpan); diff --git a/src/js/control-bar/volume-control/volume-level.js b/src/js/control-bar/volume-control/volume-level.js index 3b874c65eb..679035a21d 100644 --- a/src/js/control-bar/volume-control/volume-level.js +++ b/src/js/control-bar/volume-control/volume-level.js @@ -20,11 +20,10 @@ class VolumeLevel extends Component { const el = super.createEl('div', { className: 'vjs-volume-level' }); - const span = super.createEl('span', { - className: 'vjs-control-text' - }); - el.appendChild(span); + el.appendChild(super.createEl('span', { + className: 'vjs-control-text' + })); return el; } diff --git a/src/js/menu/menu-item.js b/src/js/menu/menu-item.js index 725a88e567..ddd2c4569a 100644 --- a/src/js/menu/menu-item.js +++ b/src/js/menu/menu-item.js @@ -68,12 +68,10 @@ class MenuItem extends ClickableComponent { tabIndex: -1 }, props), attrs); - const span = super.createEl('span', { + el.appendChild(super.createEl('span', { className: 'vjs-menu-item-text', textContent: this.localize(this.options_.label) - }); - - el.appendChild(span); + })); return el; } From cf4b38e67a45a335ee85c56a6238589e18332cc1 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Thu, 22 Jul 2021 17:32:56 -0400 Subject: [PATCH 3/8] fix role presentation --- src/js/control-bar/time-controls/time-display.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/js/control-bar/time-controls/time-display.js b/src/js/control-bar/time-controls/time-display.js index 68a3e2daf3..99999ee628 100644 --- a/src/js/control-bar/time-controls/time-display.js +++ b/src/js/control-bar/time-controls/time-display.js @@ -43,8 +43,9 @@ class TimeDisplay extends Component { }); const span = Dom.createEl('span', { className: 'vjs-control-text', - role: 'presentation', textContent: `${this.localize(this.labelText_)}\u00a0` + }, { + role: 'presentation' }); el.appendChild(span); From 48267e19716e810dd34492f5eae46967f26dfe87 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Fri, 23 Jul 2021 11:15:21 -0400 Subject: [PATCH 4/8] code review comments --- src/js/control-bar/live-display.js | 6 +++--- src/js/control-bar/spacer-controls/custom-control-spacer.js | 2 +- .../control-bar/text-track-controls/subs-caps-menu-item.js | 4 +++- src/js/control-bar/time-controls/time-divider.js | 4 +++- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/js/control-bar/live-display.js b/src/js/control-bar/live-display.js index 21e1d756a7..6a43311112 100644 --- a/src/js/control-bar/live-display.js +++ b/src/js/control-bar/live-display.js @@ -42,14 +42,14 @@ class LiveDisplay extends Component { }); this.contentEl_ = Dom.createEl('div', { - className: 'vjs-live-display', - textContent: `${this.localize('Stream Type')}\u00a0` + className: 'vjs-live-display' }, { 'aria-live': 'off' }); this.contentEl_.appendChild(Dom.createEl('span', { - className: 'vjs-control-text' + className: 'vjs-control-text', + textContent: `${this.localize('Stream Type')} ` })); this.contentEl_.appendChild(document.createTextNode(this.localize('LIVE'))); diff --git a/src/js/control-bar/spacer-controls/custom-control-spacer.js b/src/js/control-bar/spacer-controls/custom-control-spacer.js index f627c0c039..775ce7747c 100644 --- a/src/js/control-bar/spacer-controls/custom-control-spacer.js +++ b/src/js/control-bar/spacer-controls/custom-control-spacer.js @@ -32,7 +32,7 @@ class CustomControlSpacer extends Spacer { className: this.buildCSSClass(), // No-flex/table-cell mode requires there be some content // in the cell to fill the remaining space of the table. - textContent: '\u00a0' + textContent: ' ' }); } } diff --git a/src/js/control-bar/text-track-controls/subs-caps-menu-item.js b/src/js/control-bar/text-track-controls/subs-caps-menu-item.js index ee10a4fc7c..b3eaa3c91a 100644 --- a/src/js/control-bar/text-track-controls/subs-caps-menu-item.js +++ b/src/js/control-bar/text-track-controls/subs-caps-menu-item.js @@ -29,7 +29,9 @@ class SubsCapsMenuItem extends TextTrackMenuItem { })); parentSpan.appendChild(super.createEl('span', { className: 'vjs-control-text', - textContent: this.localize('Captions') + // space added as the text will visually flow with the + // label + textContent: ` ${this.localize('Captions')}` })); } diff --git a/src/js/control-bar/time-controls/time-divider.js b/src/js/control-bar/time-controls/time-divider.js index fbdaad9036..970e2d121d 100644 --- a/src/js/control-bar/time-controls/time-divider.js +++ b/src/js/control-bar/time-controls/time-divider.js @@ -28,7 +28,9 @@ class TimeDivider extends Component { }); const div = super.createEl('div'); - const span = super.createEl('span'); + const span = super.createEl('span', { + textContent: '/' + }); div.appendChild(span); el.appendChild(div); From 7931e9d85c37b731c27fbd765fb41ee8a34a1df4 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Fri, 23 Jul 2021 11:54:37 -0400 Subject: [PATCH 5/8] forgot ; --- src/js/control-bar/live-display.js | 2 +- src/js/control-bar/spacer-controls/custom-control-spacer.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/js/control-bar/live-display.js b/src/js/control-bar/live-display.js index 6a43311112..051bf94576 100644 --- a/src/js/control-bar/live-display.js +++ b/src/js/control-bar/live-display.js @@ -49,7 +49,7 @@ class LiveDisplay extends Component { this.contentEl_.appendChild(Dom.createEl('span', { className: 'vjs-control-text', - textContent: `${this.localize('Stream Type')} ` + textContent: `${this.localize('Stream Type')} ` })); this.contentEl_.appendChild(document.createTextNode(this.localize('LIVE'))); diff --git a/src/js/control-bar/spacer-controls/custom-control-spacer.js b/src/js/control-bar/spacer-controls/custom-control-spacer.js index 775ce7747c..24cd19d112 100644 --- a/src/js/control-bar/spacer-controls/custom-control-spacer.js +++ b/src/js/control-bar/spacer-controls/custom-control-spacer.js @@ -32,7 +32,7 @@ class CustomControlSpacer extends Spacer { className: this.buildCSSClass(), // No-flex/table-cell mode requires there be some content // in the cell to fill the remaining space of the table. - textContent: ' ' + textContent: ' ' }); } } From d06bc7ff69a47c8480bd53a428c228bc1d1cf6d7 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Fri, 23 Jul 2021 12:45:34 -0400 Subject: [PATCH 6/8] correct spacer textContent --- src/js/control-bar/live-display.js | 2 +- .../spacer-controls/custom-control-spacer.js | 4 ++-- src/js/control-bar/spacer-controls/spacer.js | 10 ++++++---- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/js/control-bar/live-display.js b/src/js/control-bar/live-display.js index 051bf94576..bf0a9c9408 100644 --- a/src/js/control-bar/live-display.js +++ b/src/js/control-bar/live-display.js @@ -49,7 +49,7 @@ class LiveDisplay extends Component { this.contentEl_.appendChild(Dom.createEl('span', { className: 'vjs-control-text', - textContent: `${this.localize('Stream Type')} ` + textContent: `${this.localize('Stream Type')}\u00a0` })); this.contentEl_.appendChild(document.createTextNode(this.localize('LIVE'))); diff --git a/src/js/control-bar/spacer-controls/custom-control-spacer.js b/src/js/control-bar/spacer-controls/custom-control-spacer.js index 24cd19d112..3913a8a8c0 100644 --- a/src/js/control-bar/spacer-controls/custom-control-spacer.js +++ b/src/js/control-bar/spacer-controls/custom-control-spacer.js @@ -28,11 +28,11 @@ class CustomControlSpacer extends Spacer { * The element that was created. */ createEl() { - return super.createEl({ + return super.createEl('div', { className: this.buildCSSClass(), // No-flex/table-cell mode requires there be some content // in the cell to fill the remaining space of the table. - textContent: ' ' + textContent: '\u00a0' }); } } diff --git a/src/js/control-bar/spacer-controls/spacer.js b/src/js/control-bar/spacer-controls/spacer.js index 4a8386997c..eff67f9d07 100644 --- a/src/js/control-bar/spacer-controls/spacer.js +++ b/src/js/control-bar/spacer-controls/spacer.js @@ -27,10 +27,12 @@ class Spacer extends Component { * @return {Element} * The element that was created. */ - createEl() { - return super.createEl('div', { - className: this.buildCSSClass() - }); + createEl(tag = 'div', props = {}, attributes = {}) { + if (!props.className) { + props.className = this.buildCSSClass(); + } + + return super.createEl(tag, props, attributes); } } From dac0cdd68aa4a8f6ef9a2e9f5560cb7e92370856 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Fri, 23 Jul 2021 13:30:27 -0400 Subject: [PATCH 7/8] remove children like innerHTML --- .../audio-track-controls/audio-track-menu-item.js | 5 +++++ .../text-track-controls/subs-caps-menu-item.js | 12 +++++++++--- src/js/menu/menu-item.js | 7 ++++++- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/js/control-bar/audio-track-controls/audio-track-menu-item.js b/src/js/control-bar/audio-track-controls/audio-track-menu-item.js index 7ad18a5833..72cf6450bf 100644 --- a/src/js/control-bar/audio-track-controls/audio-track-menu-item.js +++ b/src/js/control-bar/audio-track-controls/audio-track-menu-item.js @@ -47,6 +47,11 @@ class AudioTrackMenuItem extends MenuItem { createEl(type, props, attrs) { const el = super.createEl(type, props, attrs); + + while (el.firstChild) { + el.removeChild(el.firstChild); + } + const parentSpan = super.createEl('span', { className: 'vjs-menu-item-text' }); diff --git a/src/js/control-bar/text-track-controls/subs-caps-menu-item.js b/src/js/control-bar/text-track-controls/subs-caps-menu-item.js index b3eaa3c91a..6fea7056ef 100644 --- a/src/js/control-bar/text-track-controls/subs-caps-menu-item.js +++ b/src/js/control-bar/text-track-controls/subs-caps-menu-item.js @@ -4,6 +4,7 @@ import TextTrackMenuItem from './text-track-menu-item.js'; import Component from '../../component.js'; import document from 'global/document'; +import {createEl} from '../../utils/dom.js'; /** * SubsCapsMenuItem has an [cc] icon to distinguish captions from subtitles @@ -15,19 +16,24 @@ class SubsCapsMenuItem extends TextTrackMenuItem { createEl(type, props, attrs) { const el = super.createEl(type, props, attrs); - const parentSpan = super.createEl('span', { + + while (el.firstChild) { + el.removeChild(el.firstChild); + } + + const parentSpan = createEl('span', { className: 'vjs-menu-item-text' }); parentSpan.appendChild(document.createTextNode(this.localize(this.options_.label))); if (this.options_.track.kind === 'captions') { - parentSpan.appendChild(super.createEl('span', { + parentSpan.appendChild(createEl('span', { className: 'vjs-icon-placeholder' }, { 'aria-hidden': true })); - parentSpan.appendChild(super.createEl('span', { + parentSpan.appendChild(createEl('span', { className: 'vjs-control-text', // space added as the text will visually flow with the // label diff --git a/src/js/menu/menu-item.js b/src/js/menu/menu-item.js index ddd2c4569a..845802f096 100644 --- a/src/js/menu/menu-item.js +++ b/src/js/menu/menu-item.js @@ -6,6 +6,7 @@ import Component from '../component.js'; import {assign} from '../utils/obj'; import {MenuKeys} from './menu-keys.js'; import keycode from 'keycode'; +import {createEl} from '../utils/dom.js'; /** * The component for a menu item. `
  • ` @@ -68,7 +69,11 @@ class MenuItem extends ClickableComponent { tabIndex: -1 }, props), attrs); - el.appendChild(super.createEl('span', { + while (el.firstChild) { + el.removeChild(el.firstChild); + } + + el.appendChild(createEl('span', { className: 'vjs-menu-item-text', textContent: this.localize(this.options_.label) })); From e85c076f66a0863f3d7272b51694efd81a19019c Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Tue, 27 Jul 2021 14:22:47 -0400 Subject: [PATCH 8/8] do less work --- .../audio-track-controls/audio-track-menu-item.js | 14 +------------- .../text-track-controls/subs-caps-menu-item.js | 14 +------------- src/js/menu/menu-item.js | 9 +++------ 3 files changed, 5 insertions(+), 32 deletions(-) diff --git a/src/js/control-bar/audio-track-controls/audio-track-menu-item.js b/src/js/control-bar/audio-track-controls/audio-track-menu-item.js index 72cf6450bf..e8820a53be 100644 --- a/src/js/control-bar/audio-track-controls/audio-track-menu-item.js +++ b/src/js/control-bar/audio-track-controls/audio-track-menu-item.js @@ -3,7 +3,6 @@ */ import MenuItem from '../../menu/menu-item.js'; import Component from '../../component.js'; -import document from 'global/document'; /** * An {@link AudioTrack} {@link MenuItem} @@ -47,16 +46,7 @@ class AudioTrackMenuItem extends MenuItem { createEl(type, props, attrs) { const el = super.createEl(type, props, attrs); - - while (el.firstChild) { - el.removeChild(el.firstChild); - } - - const parentSpan = super.createEl('span', { - className: 'vjs-menu-item-text' - }); - - parentSpan.appendChild(document.createTextNode(this.localize(this.options_.label))); + const parentSpan = el.querySelector('.vjs-menu-item-text'); if (this.options_.track.kind === 'main-desc') { parentSpan.appendChild(super.createEl('span', { @@ -70,8 +60,6 @@ class AudioTrackMenuItem extends MenuItem { })); } - el.appendChild(parentSpan); - return el; } diff --git a/src/js/control-bar/text-track-controls/subs-caps-menu-item.js b/src/js/control-bar/text-track-controls/subs-caps-menu-item.js index 6fea7056ef..fde8f89608 100644 --- a/src/js/control-bar/text-track-controls/subs-caps-menu-item.js +++ b/src/js/control-bar/text-track-controls/subs-caps-menu-item.js @@ -3,7 +3,6 @@ */ import TextTrackMenuItem from './text-track-menu-item.js'; import Component from '../../component.js'; -import document from 'global/document'; import {createEl} from '../../utils/dom.js'; /** @@ -16,16 +15,7 @@ class SubsCapsMenuItem extends TextTrackMenuItem { createEl(type, props, attrs) { const el = super.createEl(type, props, attrs); - - while (el.firstChild) { - el.removeChild(el.firstChild); - } - - const parentSpan = createEl('span', { - className: 'vjs-menu-item-text' - }); - - parentSpan.appendChild(document.createTextNode(this.localize(this.options_.label))); + const parentSpan = el.querySelector('.vjs-menu-item-text'); if (this.options_.track.kind === 'captions') { parentSpan.appendChild(createEl('span', { @@ -41,8 +31,6 @@ class SubsCapsMenuItem extends TextTrackMenuItem { })); } - el.appendChild(parentSpan); - return el; } } diff --git a/src/js/menu/menu-item.js b/src/js/menu/menu-item.js index 845802f096..f8b660f2b9 100644 --- a/src/js/menu/menu-item.js +++ b/src/js/menu/menu-item.js @@ -69,14 +69,11 @@ class MenuItem extends ClickableComponent { tabIndex: -1 }, props), attrs); - while (el.firstChild) { - el.removeChild(el.firstChild); - } - - el.appendChild(createEl('span', { + // swap icon with menu item text. + el.replaceChild(createEl('span', { className: 'vjs-menu-item-text', textContent: this.localize(this.options_.label) - })); + }), el.querySelector('.vjs-icon-placeholder')); return el; }