Closes #4988 Update delay js script & HTML markup

[jeawhanlee]

Description

This PR updates the delay js script and set src to data-rocket-src for compatibility with update script.

Fixes #4988

Type of change

• [x] Bug fix (non-breaking change which fixes an issue)

Is the solution different from the one proposed during the grooming?

This is solution is no way different from the proposed one.

How Has This Been Tested?

Manually

Checklist:

• [x] My code follows the style guidelines of this project
• [x] I have performed a self-review of my own code
• [x] My changes generate no new warnings

[piotrbak]

As per https://github.com/wp-media/delay-javascript-loading/pull/32#issuecomment-1187002433, we need to make sure that only types mentioned Gael are rewritten to DelayJS markup.

[EDIT] I can see it being done, great!

[Mai-Saad]

@jeawhanlee @piotrbak

• We have the following regression: If we used this markup <script type='text/javascript' src='https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/scripts/print-helloworld.js'></script>, it is not delayed. which resulted in a js error while using Avada theme (this was working before)

Notes

1. if type='text/javascript' came after the script, it will be delayed i.e <script src='https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/scripts/print-helloworld.js' type='text/javascript'></script>
2. if we used before script Type='text/javascript' or TYPE='text/javascript', will be normally delayed

Question we are dealing with this markup

<script type="" src='https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/scripts/print-helloworld-v2.js'></script>
<script type src='https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/scripts/print-helloworld-v2.js'></script>

as

<script type="" src='https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/scripts/print-helloworld-v2.js'></script>
<script type="rocketlazyloadscript" type data-rocket-src='https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/scripts/print-helloworld-v2.js'></script>

shouldn't we delay 1st script and not duplicate the type in the 2nd script?

[piotrbak]

@Mai-Saad Regarding the Question, it seems to be an edge case.

Could you confirm that:

1. This markup (not cached) is valid HTML markup
2. The first script type="" is executed normally

[Mai-Saad]

As per discussion with @piotrbak and further investigations:

• The first script type="" is executed normally in the browser so probably we need to handle
• The 2nd script type, we shouldn't add another type as having 2 types will result in an error using validator HTML

[jeawhanlee]

As per discussion with @piotrbak and further investigations:

• The first script type="" is executed normally in the browser so probably we need to handle
• The 2nd script type, we shouldn't add another type as having 2 types will result in an error using validator HTML

@Mai-Saad I was not able to reproduce the duplicate type using the markup: <script type src='https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/scripts/print-helloworld-v2.js'></script>

[Mai-Saad]

@jeawhanlee @piotrbak Seems we have regression on Firefox, so far while delay JS is enabled:

• Using the bridge theme, for some reason, the home page slider is not loaded after user interaction http://mega.wp-rocket.me/bridge
• Using Divi, the page keeps loading forever after user interaction http://mega.wp-rocket.me/divi/ can you please check?

https://user-images.githubusercontent.com/76941962/191034777-e50acbea-0369-400d-bc13-5cb8405c3564.mp4

[remyperona]

PR needs to be updated with develop branch, and conflict fixed

[engahmeds3ed]

@Tabrisrp all good now, thanks for checking.

[Mai-Saad]

As per the exploratory test, we have regression on FF mobile view => page keeps loading and has many warnings while a separate mobile cache is enabled/disabled and Delay JS has no exclusions. @engahmeds3ed @jeawhanlee can you please check :pray:

https://user-images.githubusercontent.com/76941962/205810699-a8b1450d-7da8-448c-943e-b7c2720bdc01.mp4

Note: Same is happening on mega/divi (working fine on the trunk)

[piotrbak]

For Avada example, excluding the following scripts makes slider appear right away. The root cause is most likely inside them:

/wp-content/themes/Avada/includes/lib/assets/min/js/library/jquery.flexslider.js
/avada/wp-includes/js/jquery/jquery.min.js
/wp-content/plugins/fusion-core/js/min/avada-fusion-slider.js
/wp-content/themes/Avada/includes/lib/assets/min/js/library/modernizr.js
/wp-content/themes/Avada/includes/lib/assets/min/js/general/fusion.js
/wp-content/themes/Avada/includes/lib/assets/min/js/library/cssua.js
/wp-content/themes/Avada/includes/lib/assets/min/js/library/bootstrap.transition.js
/wp-content/themes/Avada/includes/lib/assets/min/js/library/bootstrap.tooltip.js

[gmetais]

As per the exploratory test, we have regression on FF mobile view

@Mai-Saad @piotrbak Actually, this is not really a bug.

We used to have an issue on Firefox, which led me to implement a Firefox browser detection in the code by checking the user agent. But when you switch Firefox to mobile view, it overwrites the user agent and replaces it with an Android or iOS user agent.

It should not reproduce on a real Firefox mobile browser. If you still want to test, Firefox allows you to edit the user agent. Adding "Firefox/107.0" to the end of the user agent field fixes the issue.

In case you still consider this issue as blocking (for example if some website creators use the Firefox mobile view to debug their website), we could try to find a different way to identify Firefox. For example, on Firefox, navigator.vendor === "" whereas other browsers have non-empty values. Kind of a hack and I prefer not to add it.