Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dispose() gives error: Cannot read property 'classList' of null #5706

Closed
DimaM-BeStreet opened this issue Dec 23, 2018 · 25 comments
Closed

dispose() gives error: Cannot read property 'classList' of null #5706

DimaM-BeStreet opened this issue Dec 23, 2018 · 25 comments

Comments

@DimaM-BeStreet
Copy link

DimaM-BeStreet commented Dec 23, 2018

Description

Hey guys, I've been going crazy for a full day over this bug, thinking i must be missing something (i still might be)
basically I get an error every time I use dispose, including this super simple reduced case:
https://codepen.io/dimam-bestreet/pen/GPmoxX

Steps to reproduce

  1. add dynamically a video
  2. run .dispose on it
  3. look in the chrome (Chromium 65) dev-tools for this error:
    VM76237:214 Uncaught TypeError: Cannot read property 'classList' of null
    at VideojsDelegate (:214:46)
    at add_element (:395:17)
    at MutationObserver. (:419:17)

Results

Expected

no error..

Actual

error... ^^

Error output

If there are any errors at all, please include them here.
VM76237:214 Uncaught TypeError: Cannot read property 'classList' of null
at VideojsDelegate (:214:46)
at add_element (:395:17)
at MutationObserver. (:419:17)

Additional Information

I've checked in firefox - no error!
I am using crome (Chromium 65) on linux (if that has anything to do with anything)

versions

videojs

7.3.0

browsers

Chromium 65, firefox seems ok

OSes

I've checked only mine - linux mint 17.3

plugins

nope

Is this is only related to the fact im using Chromium 65??

@welcome
Copy link

welcome bot commented Dec 23, 2018

👋 Thanks for opening your first issue here! 👋

If you're reporting a 🐞 bug, please make sure you include steps to reproduce it. We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.
To help make it easier for us to investigate your issue, please follow the contributing guidelines.

@gkatsev
Copy link
Member

gkatsev commented Dec 26, 2018

Unfortunately, I'm unable to reproduce locally or in browserstack on chrome 65 on win10 or macos.

@DimaM-BeStreet
Copy link
Author

DimaM-BeStreet commented Jan 2, 2019 via email

@thijstriemstra
Copy link
Contributor

I think i've seen this in a unit test before (so don't wait for ready event but dispose right away or something like that).

@stale
Copy link

stale bot commented Mar 14, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the outdated Things closed automatically by stalebot label Mar 14, 2019
@stale stale bot closed this as completed Mar 21, 2019
@armaha
Copy link

armaha commented Apr 11, 2019

Hi,
I also notice the same issue, with almost the exact same stack trace for VideoJs 6.6. It also only occurs in Chrome. We block our instance from calling dispose if not ready like this:

if(this.player) {
    this.player.dispose();
}

We instantiate the player like this:

const nonReadyPlayer = videojs(
                    this.videoId,
                    this.videoConfig,
                    () => {
                        this.player = nonReadyPlayer;
                    }
);

As we cannot reproduce this exception, it is difficult to pinpoint why this is occurring in a very small number of instances ( n<0.01% of requests). I also cannot find any references to VideoJsDelegate MutationObserver in VideoJs release 6.6.

Is there any specific behavior for autoplay that needs to be guarded against in chrome?
Where does VideoJSDelegate come from if not from video.js repo?

Thanks!

@travisbader
Copy link
Contributor

I think i found a good stack trace. I will attach as our bug tracking too reports it as it is easy to read as formatted.

Screen Shot 2020-06-08 at 8 27 52 AM

@thijstriemstra
Copy link
Contributor

@gkatsev can this be re-opened? A check for element seems to be needed in dom.js:

if (element && element.classList) {

@gkatsev gkatsev reopened this Jun 19, 2020
@stale stale bot removed the outdated Things closed automatically by stalebot label Jun 19, 2020
@gkatsev
Copy link
Member

gkatsev commented Jun 19, 2020

I'm not seeing an error locally.
While I'm not necessarily opposed to adding a null check to that method but almost always there's something else that needs to be fixed in cases like this.

@stale
Copy link

stale bot commented Aug 22, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the outdated Things closed automatically by stalebot label Aug 22, 2020
@thijstriemstra
Copy link
Contributor

While I'm not necessarily opposed to adding a null check to that method but almost always there's something else that needs to be fixed in cases like this.

Can you re-open this ticket @gkatsev?

@stale stale bot removed the outdated Things closed automatically by stalebot label Aug 23, 2020
@gkatsev
Copy link
Member

gkatsev commented Aug 27, 2020

Anyone have info on how to reproduce this issue?

@travisbader
Copy link
Contributor

@gkatsev See above, there are steps to reproduce in the initial description. It appears to be related to disposing and re-creating video-js objects within a component

@gkatsev
Copy link
Member

gkatsev commented Aug 27, 2020

As I mentioned above, I'm unable to reproduce with either the test page or locally.

@thijstriemstra
Copy link
Contributor

As I mentioned above, I'm unable to reproduce with either the test page or locally.

I wasn't able to reproduce it with that test page either, but while playing with it, I was able to break it using:

      videojs("my_video", {}, function ready() {
        console.log("ready");
        videojs("my_video").dispose();
      });

I don't think it should throw this/any error:

Screenshot from 2020-08-27 19-05-26

@gkatsev
Copy link
Member

gkatsev commented Aug 27, 2020

Looks like a different error but related to disposing. It probably should handle that case gracefully. Probably a timing issue with preselectTrack there.

@thijstriemstra
Copy link
Contributor

I agree. Wouldn't surprise me that fixing this issue reveals the other issue..

@damjan25
Copy link

damjan25 commented Oct 22, 2020

@gkatsev
Is there a plan to release this in 7.10.2 ? #6701
Would be really nice. I am seeing this error a lot lately.

(Using videojs + videojsima plugin)
This error gets thrown when I get AdsLoader error: AdError 303: No Ads VAST response after one or more Wrappers
And after player tries to dispose, this error(Cannot read property 'classList' of null) gets thrown.
But for some reason only when I hard reload the page.

@gkatsev
Copy link
Member

gkatsev commented Dec 11, 2020

We just merged #6967, hopefully that should fix some of the issues related to this.

@thijstriemstra
Copy link
Contributor

#6701 was merged and I think this can be closed.

@gkatsev
Copy link
Member

gkatsev commented Jul 24, 2021

Thanks for following up. Forgot to close it as part of the commit.

@gkatsev gkatsev closed this as completed Jul 24, 2021
@gkatsev
Copy link
Member

gkatsev commented Jul 24, 2021

Fix is in 7.14.2.

@arifuxd
Copy link

arifuxd commented Oct 30, 2021

Fix is in 7.14.2.

No This is not fixed. I am currently getting it in 7.15.4

image

@damjan25
Copy link

Still have this error :/

@mister-ben
Copy link
Contributor

The issue demonstrated in the test case was fixed when this was closed. If you are seeing an issue in a different scenario, please open a new issue with steps to reproduce.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants