dnnsoftware / Dnn.Platform

DNN (formerly DotNetNuke) is the leading open source web content management platform (CMS) in the Microsoft ecosystem.
https://dnncommunity.org/
MIT License
1.01k stars 745 forks source link

[Bug]: Registers an unload listener #5913

Open GaurangMistry opened 5 months ago

GaurangMistry commented 5 months ago

Is there an existing issue for this?

What happened?

We have many sites in DNN and we are creating all sites perfectly with Google LightHouse. In Google LightHouse, we are facing one issue common in all sites is "Registers an unload listener". This is coming from Default Jquery file which DNN used.

I searched in google and got many suggestions to delete "Unload" from addEventListener method from Jquery or use "visibilitychange" but it's not work. We also take help of Jquery experts but not get expected result. So, we wish if you have any idea about it and you can directly work with us on project to fix this issue.

Steps to reproduce?

  1. Install Google Chrome Light House plugin
  2. Open website: https://www.shopsmart.website/ in Google Chrome
  3. Right-Click on website and select "Inspect".
  4. There is one option called "Lighthouse". Select it and click on "Analyze page load".
  5. Select "Best Practices" and you will get error for "Registers an unload listener".

Current Behavior

2024-01-10_16-09-04 2024-01-10_16-08-40 2024-01-10_16-07-58

Expected Behavior

Need to fix "Registers an unload listener" error and get 100% score in Google Lighthouse

Relevant log output

I searched in google and got many suggestions to delete "Unload" from addEventListener method from Jquery or use "visibilitychange" but it's not work. We also take help of Jquery experts but not get expected result.

Anything else?

No response

Affected Versions

9.13.0 (latest release)

What browsers are you seeing the problem on?

Firefox, Chrome, Safari, Microsoft Edge

Code of Conduct

valadas commented 5 months ago

I am not sure if this is a DNN issue or not. If jQuery just does it, it is out of our purfiew. JQuery is not required in DNN for visitors, only for the edit experience (with the exception of popups being enabled and the Search skinobject).

We can't really troubleshoot from a production site with using the client resource management.

The current out-of-the-box theme does provide jQuery but the new theme in DNN10 will not. At any rate, your theme is not the out-of-box one so it is a moot point. But for now this is not really actionable unless there is reasons the believe this is DNN specific. For what its worth this error/warning is also present if you inspect github right here :)

If you have more details to make this issue actionable, please add info and I'll be happy to reopen.

uzmannazari commented 3 months ago

hello valadas i thinks this is not the issue for jquery or ajax. this is the problem that dnn used unload listeners on backend i think! there is way to use beforeunload to fix this problem but i dont know how to change and where the functions used this

uzmannazari commented 3 months ago

but this is the big problem that dnn websites are now have low rate of lighthouse because of this problem

valadas commented 3 months ago

From what I can see, it looks like a Microsoft.ajas issue and you already opened an issue there too https://github.com/MicrosoftDocs/feedback/issues/3975

valadas commented 3 months ago

Searching the codebase here, I found this: https://github.com/dnnsoftware/Dnn.Platform/blob/463b867bcdd850aa9bb6a581f6d8b4da59b7f648/DNN%20Platform/Website/js/Debug/MicrosoftAjaxWebForms.js#L1181 which may or may not be related

6TELOIV commented 3 months ago

The unload handler is being set here:

https://github.com/dnnsoftware/Dnn.Platform/blob/463b867bcdd850aa9bb6a581f6d8b4da59b7f648/DNN%20Platform/Website/js/Debug/MicrosoftAjax.js#L3592

6TELOIV commented 3 months ago

It appears to be a MicrosoftAjax issue, but no idea where an appropriate place would be to open an issue with them.

valadas commented 3 months ago

Well, the link above is in DNN, I don't know off-the-cuff if it is the only instance, but it would be a start... I am not sure the error is on Microsoft side now with searching here...

uzmannazari commented 3 months ago

as i see this is to replace the unload with beforeunload function but i dont know what will happen if we do that on dnn backend and whats the usage of this function !

6TELOIV commented 3 months ago

That could be something to try out and just see what happens; modify this line to say beforeunload instead of unload, though that probably won't work if the event actually needs to fire because beforeunload events won't run code.

I would recommend trying pagehide, or a more complex change would be visibilitychange because you'd have to check if the visibility is now "hidden" before firing their event. See this Google Dev article for more info.

6TELOIV commented 3 months ago

Another thing to try is just delete the line entirely and see what happens. It already doesn't fire reliably, and it will be gone for good eventually, so it would be good to see what happens if it just wasn't there at all.

uzmannazari commented 3 months ago

i changed the MicrosoftAjax.js file yesterday but the scriptresource.axd file didnt changed! i dont know how to change this ScriptResource.axd file and how this file is being created!

valadas commented 3 months ago

scriptresource.axd is a handler that can serve multiple scripts potentially merged together and/or minified, it is not a file exactly. What needs to be done is to see what is using onload, figure out the effects of changing it, etc.

valadas commented 3 months ago

Also, I spotted by a search one usage, but there could be more to it...

uzmannazari commented 3 months ago

great @GaurangMistry thanks for share this. but how can we set this condition on default.aspx file?

uzmannazari commented 3 months ago

Untitled i have this error when set this condition on default.aspx file

valadas commented 3 months ago

I don't think this is the proper fix. not using the offending method would be a better fix. The code is compiled into an assembly. There could be some workaround but the proper fix is to find the correct source of the bug.

uzmannazari commented 3 months ago

I checked the @GaurangMistry website and now there is no error for unload listener on his website!

uzmannazari commented 3 months ago

@valadas , this error on google lighthouse is not bug! it is about the standards that to be better on google websites need that.

We should not use unload listener in our codes

valadas commented 3 months ago

I agree, my point is that preventing the ajax library from loading is more of a workaround than a proper fix. I may have ramifications and the proper fix would be to fix the underlying code to not use unload.

uzmannazari commented 3 months ago

your right mr valadas, now what should we do to fix this problem with lighthouse? should we edit microsoft ajax js or dnn core?

valadas commented 3 months ago

I am not sure without spending the time looking into it. I think in DNN usage of microsoft Ajax potentially.