Delay JS - TrustPilot additional scripts

webtrainingwheels commented 4 years ago

Trustpilot is in our default list of "safe to delay" scripts.

A customer is using the Trustpilot plugin: https://wordpress.org/plugins/trustpilot-reviews/

Turns out, this plugin loads its files everywhere, and even without authenticating a TrustPilot account.🙃 So, to reproduce:

Activate the plugin on a site (no need to connect to a TrustPilot account)
Activate Delay JS with default list
Visit any page. You'll see numerous JS errors in the console:https://jmp.sh/0n8Wf12
Using Vasilis' list of additional scripts to delay from his comment, will resolve the errors.

Plugin is installed on Smashing Coding.

Ticket: https://secure.helpscout.net/conversation/1277705875/193437?folderId=377611

I could not test this on Mega because the Trustpilot plugin requires an https connection.

Backlog Grooming (for WP Media dev team use only)

[x] Reproduce the problem
[x] Identify the root cause
[x] Scope a solution
[x] Estimate the effort

webtrainingwheels commented 4 years ago

After some further testing on the customer's site, there was an issue related to this script: plugins/trustpilot-reviews/review/assets/js/trustBoxScript.js However I couldn't actually find that script in the source. In the end I removed all trustpilot scripts from delaying, including our default, widget.trustpilot.com

arunbasillal commented 4 years ago

@webtrainingwheels I have requested for SSL on Slack to JB. When available, hope you can try this out and add the steps to reproduce 🙏

vmanthos commented 4 years ago

In this ticket: https://secure.helpscout.net/conversation/1279236604/193990?folderId=2135277

I had to delay the following to prevent a trustpilot_settings not defined error:

trustpilot_settings
invitejs.trustpilot.com/tp.min.js
trustbox_settings
/wp-content/plugins/trustpilot-reviews/review/assets/js/trustBoxScript.min.js
/wp-content/plugins/trustpilot-reviews/review/assets/js/headerScript.min.js

They are using the official Trustpilot plugin.

webtrainingwheels commented 4 years ago

@arunbasillal I have updated my original post with the steps

jorgemartine00 commented 4 years ago

This ticket can be taken as an example too https://secure.helpscout.net/conversation/1280991905/194504/ Exclude files from Defer JS helper plugin was used to delay the following scripts and that worked for the client: /wp-content/plugins/trustpilot-reviews/review/assets/js/trustBoxScript.min.js /wp-content/plugins/trustpilot-reviews/review/assets/js/headerScript.min.js

remyperona commented 4 years ago

Reproduce the issue

Reproduced with the plugin on my local installation.

Identify the root cause

This is caused by the pattern widget.trustpilot.com, because it is matching the 3rd party JS URL (//widget.trustpilot.com/bootstrap/v5/tp.widget.bootstrap.min.js), but it's also matching inside one of the inline script added by the trustpilot plugin:

var trustpilot_settings = {
    key: "",
    TrustpilotScriptUrl: "https:\/\/invitejs.trustpilot.com\/tp.min.js",
    IntegrationAppUrl: "\/\/ecommscript-integrationapp.trustpilot.com",
    PreviewScriptUrl: "\/\/ecommplugins-scripts.trustpilot.com\/v2.1\/js\/preview.min.js",
    PreviewCssUrl: "\/\/ecommplugins-scripts.trustpilot.com\/v2.1\/css\/preview.min.css",
    PreviewWPCssUrl: "\/\/ecommplugins-scripts.trustpilot.com\/v2.1\/css\/preview_wp.css"",
    WidgetScriptUrl: "\/\/widget.trustpilot.com\/bootstrap\/v5\/tp.widget.bootstrap.min.js",
};

This is a side-effect of the pattern, creating a number of JS issues because other scripts are depending on this inline JS variable to work.

Scope a solution

There is no reason to delay the above inline script, as it has no impact on the loading time of the page. Ideally, we should update the default pattern widget.trustpilot.com to something more specific, to make sure it only matches with the actual URL for the widget, and nothing else.

A solution could be to use a pattern like the following widget.trustpilot.com/bootstrap/v5/tp.widget.bootstrap.min.js instead. This would not match the inline script, as the script is using escaping.

Estimate the effort

Effort [S]

GeekPress commented 4 years ago

It seems the problem is to not target // in our default pattern. If we keep the full URL, we will have to update it each time the version number will be updated. Trustpilot is adding the version number on all their files into the URL.

What about //widget.trustpilot.com/bootstrap/ for our default list?

remyperona commented 4 years ago

We can do that, we have to update our sanitization because currently // is removed when it's part of the input

GeekPress commented 4 years ago

And if we do widget.trustpilot.com/bootstrap/, will it require an extra work?

remyperona commented 4 years ago

This can work 👍🏼

wp-media / wp-rocket