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

Using window.onload :( #882

Closed
demon-ru opened this issue May 15, 2017 · 7 comments

Comments

Projects
None yet
4 participants
@demon-ru
Copy link

commented May 15, 2017

After some updates I have a bug: upload container does not visible after click.
The reason is using window.onload in app/assets/js/rtMedia.js
And other (theme) using it in js code.

I think this code should be replaced by something more secure.

@pranali333

This comment has been minimized.

Copy link
Contributor

commented May 16, 2017

Hello,

Can you please explain your issue in more detail?
Kindly, provide some screenshots and the debug info log on your website. You can find it under rtMedia Settings -> Supports tab. We will regenerate the issue on our end to make necessary improvements.

Thanks.

@demon-ru

This comment has been minimized.

Copy link
Author

commented May 16, 2017

more detail.

there are:

  1. one plugin with JS using "window.onload = function1;"
  2. one theme (or other plugin) with JS using "window.onload = function2;"
    in my case: plugin buddypress-media and theme suffusion

Case:
What is the value of window.onload after simultaneous loaded these two JS?
Yes. It depends on what script last set value.
The first set will not get executed handler.

Some plugins use the way: save the old value before setting handler and call it into function.
The best way: to find another way. Why the presence of jQuery( document ).ready() is additionally used window.onload?

I hope I could explain using clumsy English :)

@demon-ru

This comment has been minimized.

Copy link
Author

commented May 16, 2017

@pranali333

This comment has been minimized.

Copy link
Contributor

commented May 16, 2017

Hello,

Thanks for the explanation. Yes, we got your issue correctly and we will look into it.

I will update you soon regarding this.

Thanks for reporting this to us.

@BhargavBhandari90 BhargavBhandari90 added this to the v4.4 milestone May 16, 2017

@BhargavBhandari90

This comment has been minimized.

Copy link
Contributor

commented May 27, 2017

@demon-ru Can you confirm with the PR:#899

BhargavBhandari90 pushed a commit to BhargavBhandari90/rtMedia that referenced this issue May 31, 2017

BhargavBhandari90 added a commit that referenced this issue May 31, 2017

@Sidsector9 Sidsector9 added the fixed label Jun 2, 2017

@demon-ru

This comment has been minimized.

Copy link
Author

commented Jun 8, 2017

After plugin update:

  1. upload container visible after click. Bug fixed
  2. window.onload is still using in app/assets/js/rtMedia.js without save prior value.
    I guess it will be troubles in future. At least with theme suffusion.
@BhargavBhandari90

This comment has been minimized.

Copy link
Contributor

commented Jun 17, 2017

Hi @demon-ru

We checked with all default themes and there is not any issue with it. In fact, no other themes have this kind of issue.

We can consider your suggestion in future.

Or it will be good if you can send PR for this so that we can discuss that more deeply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.