cleanup browser tests and fix null reference check on window.documentElement.style.WebkitAppearance #447
Conversation
…Element.style.WebkitAppearance
thebigredgeek
added
bug
|
@TooTallNate can you do a sanity check on this for me? |
|
also fixes #436 |
|
@thebigredgeek I think this has broken webworker compatibility (introduced by #382) |
|
@darrachequesne whoops! That was sloppy of me! I knew I should have waited for the sanity check from @TooTallNate haha. I just deprecated 2.6.5 and released 2.6.6 with a fix. You should be able to unpin |
|
@thebigredgeek great, thanks! |
| @@ -40,20 +40,20 @@ function useColors() { | |||
| // NB: In an Electron preload script, document will be defined but not fully | |||
| // initialized. Since we know we're in Chrome, we'll just detect this case | |||
| // explicitly | |||
| if (typeof window !== 'undefined' && window && typeof window.process !== 'undefined' && window.process.type === 'renderer') { | |||
| if (window && window.process && window.process.type === 'renderer') { | |||
There was a problem hiding this comment.
The typeof window !== 'undefined' stuff is important, because that will return false if window is not defined, but simply window will throw an error if it is not defined. All of the root-level variables should stick with the typeof check IMO. Once that first check passes then checking for thuthyness should be enough for the properties after that.
| return true; | ||
| } | ||
|
|
||
| // is webkit? http://stackoverflow.com/a/16459606/376773 | ||
| // document is undefined in react-native: https://github.com/facebook/react-native/pull/1632 | ||
| return (typeof document !== 'undefined' && document && 'WebkitAppearance' in document.documentElement.style) || | ||
| return (document && document.documentElement && document.documentElement.style && document.documentElement.style.WebkitAppearance) || |
There was a problem hiding this comment.
This is the only really important line it seems like. For the purposes of fixing #436, maybe we can narrow the bug fix commit to just a one-liner commit. And perhaps address other styling fixes for this code separately.
|
Damnit, this GitHub Reviews feature tripped me up. First time using it. I had these comments written out 2 days ago and I thought they were visible. Turns out I had to hit the "Submit Review" button first. My bad. |
No description provided.