From c62de28e3ba97cb8579ae5ac616e368c370baaf6 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Mon, 3 Oct 2016 16:57:15 -0400 Subject: [PATCH 1/2] feat: add a safe getComputedStyle to videojs. This is used internally in the seek bar and mouse time display. In firefox, in an iframe that is hidden with "display: none", getComputedStyle() returns "null" which can break things. See https://bugzilla.mozilla.org/show_bug.cgi?id=548397 for more details. --- .../progress-control/mouse-time-display.js | 8 +++---- .../control-bar/progress-control/seek-bar.js | 6 +++--- src/js/utils/get-computed-style.js | 21 +++++++++++++++++++ src/js/video.js | 15 +++++++++++++ 4 files changed, 43 insertions(+), 7 deletions(-) create mode 100644 src/js/utils/get-computed-style.js diff --git a/src/js/control-bar/progress-control/mouse-time-display.js b/src/js/control-bar/progress-control/mouse-time-display.js index c72d132adf..aeaeff2db5 100644 --- a/src/js/control-bar/progress-control/mouse-time-display.js +++ b/src/js/control-bar/progress-control/mouse-time-display.js @@ -1,12 +1,12 @@ /** * @file mouse-time-display.js */ -import window from 'global/window'; import Component from '../../component.js'; import * as Dom from '../../utils/dom.js'; import * as Fn from '../../utils/fn.js'; import formatTime from '../../utils/format-time.js'; import throttle from 'lodash-compat/function/throttle'; +import computedStyle from '../../utils/get-computed-style.js'; /** * The Mouse Time Display component shows the time you will seek to @@ -71,7 +71,7 @@ class MouseTimeDisplay extends Component { if (this.keepTooltipsInside) { const clampedPosition = this.clampPosition_(position); const difference = position - clampedPosition + 1; - const tooltipWidth = parseFloat(window.getComputedStyle(this.tooltip).width); + const tooltipWidth = parseFloat(computedStyle(this.tooltip, 'width')); const tooltipWidthHalf = tooltipWidth / 2; this.tooltip.innerHTML = time; @@ -98,8 +98,8 @@ class MouseTimeDisplay extends Component { return position; } - const playerWidth = parseFloat(window.getComputedStyle(this.player().el()).width); - const tooltipWidth = parseFloat(window.getComputedStyle(this.tooltip).width); + const playerWidth = parseFloat(computedStyle(this.player().el(), 'width')); + const tooltipWidth = parseFloat(computedStyle(this.tooltip, 'width')); const tooltipWidthHalf = tooltipWidth / 2; let actualPosition = position; diff --git a/src/js/control-bar/progress-control/seek-bar.js b/src/js/control-bar/progress-control/seek-bar.js index 8fa34694a6..416e025baf 100644 --- a/src/js/control-bar/progress-control/seek-bar.js +++ b/src/js/control-bar/progress-control/seek-bar.js @@ -1,11 +1,11 @@ /** * @file seek-bar.js */ -import window from 'global/window'; import Slider from '../../slider/slider.js'; import Component from '../../component.js'; import * as Fn from '../../utils/fn.js'; import formatTime from '../../utils/format-time.js'; +import computedStyle from '../../utils/get-computed-style.js'; import './load-progress-bar.js'; import './play-progress-bar.js'; @@ -65,8 +65,8 @@ class SeekBar extends Slider { this.updateAriaAttributes(this.tooltipProgressBar.el_); this.tooltipProgressBar.el_.style.width = this.bar.el_.style.width; - const playerWidth = parseFloat(window.getComputedStyle(this.player().el()).width); - const tooltipWidth = parseFloat(window.getComputedStyle(this.tooltipProgressBar.tooltip).width); + const playerWidth = parseFloat(computedStyle(this.player().el(), 'width')); + const tooltipWidth = parseFloat(computedStyle(this.tooltipProgressBar.tooltip, 'width')); const tooltipStyle = this.tooltipProgressBar.el().style; tooltipStyle.maxWidth = Math.floor(playerWidth - (tooltipWidth / 2)) + 'px'; diff --git a/src/js/utils/get-computed-style.js b/src/js/utils/get-computed-style.js new file mode 100644 index 0000000000..57ca745066 --- /dev/null +++ b/src/js/utils/get-computed-style.js @@ -0,0 +1,21 @@ +/** + * A safe getComputedStyle with an IE8 fallback. + * + * This is because in Firefox, if the player is loaded in an iframe with `display:none`, + * then `getComputedStyle` returns `null`, so, we do a null-check to make sure + * that the player doesn't break in these cases. + * See https://bugzilla.mozilla.org/show_bug.cgi?id=548397 for more details. + * + * @function getComputedStyle + * @param el the element you want the computed style of + * @param prop the property name you want + */ +export default function getComputedStyle(el, prop) { + if (typeof getComputedStyle === 'function') { + const cs = getComputedStyle(el); + + return cs ? cs[prop] : ''; + } + + return el.currentStyle[prop]; +} diff --git a/src/js/video.js b/src/js/video.js index 28638cd153..0cc8708f6a 100644 --- a/src/js/video.js +++ b/src/js/video.js @@ -25,6 +25,7 @@ import log from './utils/log.js'; import * as Dom from './utils/dom.js'; import * as browser from './utils/browser.js'; import * as Url from './utils/url.js'; +import computedStyle from './utils/get-computed-style.js'; import extendFn from './extend.js'; import merge from 'lodash-compat/object/merge'; import xhr from 'xhr'; @@ -719,6 +720,20 @@ videojs.appendContent = Dom.appendContent; */ videojs.insertContent = Dom.insertContent; +/** + * A safe getComputedStyle with an IE8 fallback. + * + * This is because in Firefox, if the player is loaded in an iframe with `display:none`, + * then `getComputedStyle` returns `null`, so, we do a null-check to make sure + * that the player doesn't break in these cases. + * See https://bugzilla.mozilla.org/show_bug.cgi?id=548397 for more details. + * + * @function getComputedStyle + * @param el the element you want the computed style of + * @param prop the property name you want + */ +videojs.getComputedStyle = computedStyle; + /* * Custom Universal Module Definition (UMD) * From 0d15f4b39c31d77fda09a45b4acf925596db867e Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Tue, 4 Oct 2016 11:50:46 -0400 Subject: [PATCH 2/2] Rename getComputedStyle to computedStyle. Null check el/prop --- .../progress-control/mouse-time-display.js | 2 +- src/js/control-bar/progress-control/seek-bar.js | 2 +- .../{get-computed-style.js => computed-style.js} | 16 +++++++++++----- src/js/video.js | 6 +++--- 4 files changed, 16 insertions(+), 10 deletions(-) rename src/js/utils/{get-computed-style.js => computed-style.js} (62%) diff --git a/src/js/control-bar/progress-control/mouse-time-display.js b/src/js/control-bar/progress-control/mouse-time-display.js index aeaeff2db5..129a2b8603 100644 --- a/src/js/control-bar/progress-control/mouse-time-display.js +++ b/src/js/control-bar/progress-control/mouse-time-display.js @@ -6,7 +6,7 @@ import * as Dom from '../../utils/dom.js'; import * as Fn from '../../utils/fn.js'; import formatTime from '../../utils/format-time.js'; import throttle from 'lodash-compat/function/throttle'; -import computedStyle from '../../utils/get-computed-style.js'; +import computedStyle from '../../utils/computed-style.js'; /** * The Mouse Time Display component shows the time you will seek to diff --git a/src/js/control-bar/progress-control/seek-bar.js b/src/js/control-bar/progress-control/seek-bar.js index 416e025baf..b46e4189cb 100644 --- a/src/js/control-bar/progress-control/seek-bar.js +++ b/src/js/control-bar/progress-control/seek-bar.js @@ -5,7 +5,7 @@ import Slider from '../../slider/slider.js'; import Component from '../../component.js'; import * as Fn from '../../utils/fn.js'; import formatTime from '../../utils/format-time.js'; -import computedStyle from '../../utils/get-computed-style.js'; +import computedStyle from '../../utils/computed-style.js'; import './load-progress-bar.js'; import './play-progress-bar.js'; diff --git a/src/js/utils/get-computed-style.js b/src/js/utils/computed-style.js similarity index 62% rename from src/js/utils/get-computed-style.js rename to src/js/utils/computed-style.js index 57ca745066..f2edbba33a 100644 --- a/src/js/utils/get-computed-style.js +++ b/src/js/utils/computed-style.js @@ -1,3 +1,5 @@ +import window from 'global/window'; + /** * A safe getComputedStyle with an IE8 fallback. * @@ -6,16 +8,20 @@ * that the player doesn't break in these cases. * See https://bugzilla.mozilla.org/show_bug.cgi?id=548397 for more details. * - * @function getComputedStyle + * @function computedStyle * @param el the element you want the computed style of * @param prop the property name you want */ -export default function getComputedStyle(el, prop) { - if (typeof getComputedStyle === 'function') { - const cs = getComputedStyle(el); +export default function computedStyle(el, prop) { + if (!el || !prop) { + return ''; + } + + if (typeof window.getComputedStyle === 'function') { + const cs = window.getComputedStyle(el); return cs ? cs[prop] : ''; } - return el.currentStyle[prop]; + return el.currentStyle[prop] || ''; } diff --git a/src/js/video.js b/src/js/video.js index 0cc8708f6a..7ed4d2307e 100644 --- a/src/js/video.js +++ b/src/js/video.js @@ -25,7 +25,7 @@ import log from './utils/log.js'; import * as Dom from './utils/dom.js'; import * as browser from './utils/browser.js'; import * as Url from './utils/url.js'; -import computedStyle from './utils/get-computed-style.js'; +import computedStyle from './utils/computed-style.js'; import extendFn from './extend.js'; import merge from 'lodash-compat/object/merge'; import xhr from 'xhr'; @@ -728,11 +728,11 @@ videojs.insertContent = Dom.insertContent; * that the player doesn't break in these cases. * See https://bugzilla.mozilla.org/show_bug.cgi?id=548397 for more details. * - * @function getComputedStyle + * @function computedStyle * @param el the element you want the computed style of * @param prop the property name you want */ -videojs.getComputedStyle = computedStyle; +videojs.computedStyle = computedStyle; /* * Custom Universal Module Definition (UMD)