feat: add a safe getComputedStyle to videojs. #3664
Conversation
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.
gkatsev
force-pushed
the
safe-get-computed-style
branch
from
40f0248 to
c62de28
Compare
| * @param prop the property name you want | ||
| */ | ||
| export default function getComputedStyle(el, prop) { | ||
| if (typeof getComputedStyle === 'function') { |
There was a problem hiding this comment.
seems like we may want to check for el or prop being null here and return empty sting if that happens.
There was a problem hiding this comment.
Also, shouldn't this be window.getComputedStyle?
I think this'll detect the module-level function instead of the global one.
There was a problem hiding this comment.
Yeah, it'll be safer to do window..
I guess checking that el and prop are available is good. I'll refrain from doing other type checking, though.
| * @param el the element you want the computed style of | ||
| * @param prop the property name you want | ||
| */ | ||
| videojs.getComputedStyle = computedStyle; |
There was a problem hiding this comment.
do we want to call it getComputedStyle? Should it be getComputedStyle throughout the code rather than computedStyle?
There was a problem hiding this comment.
I think it should be computedStyle so we don't confuse it with the global function.
|
|
||
| return cs ? cs[prop] : ''; | ||
| } | ||
|
|
||
| return el.currentStyle[prop]; | ||
| return el.currentStyle[prop] || ''; |
misteroneill
added
confirmed
minor
This will prevent a null exception when a video.js is implemented in a 'display:none' iframe on Firefox (<62). This is a fix in continuation of the PR #3664 regarding a bug in Firefox https://bugzilla.mozilla.org/show_bug.cgi?id=548397
Description
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.
Specific Changes proposed
getComputedStyleutilvideojs.getComputedStyle