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

3.8 - JavaScript deferring breaks Smart Slider 3 #3421

Closed
4 tasks done
vmanthos opened this issue Dec 17, 2020 · 2 comments · Fixed by #3424
Closed
4 tasks done

3.8 - JavaScript deferring breaks Smart Slider 3 #3421

vmanthos opened this issue Dec 17, 2020 · 2 comments · Fixed by #3424
Assignees
Labels
3rd party compatibility Issues related to 3rd party compatibility like theme, plugin or hosting effort: [XS] < 1 day of estimated development time module: file optimization type: bug Indicates an unexpected problem or unintended behavior
Milestone

Comments

@vmanthos
Copy link
Contributor

vmanthos commented Dec 17, 2020

Before submitting an issue please check that you’ve completed the following steps:

  • Made sure you’re on the latest version ✅
  • Used the search feature to ensure that the bug hasn’t been reported before ✅

Describe the bug

With WP Rocket 3.8, NOT deferring jQuery results into Smart Slider's inline JavaScript changing scope:

The solution is either to prevent jQuery from being deferred or exclude the inline JavaScript from being deferred:

add_filter( 'rocket_defer_inline_exclusions', 'N.N2_');

To Reproduce

Steps to reproduce the behavior:

  1. Add a Smart Slider on a page.
  2. Defer the jQuery library.
  3. The slider won't be displayed.

Expected behavior

The slider should be displayed.

Additional context

Should we add an array to store these exclusions, the way we do with inline JavaScript to prevent the cache dir size issue?

$inline_exclusions = apply_filters( 'rocket_defer_inline_exclusions', 'DOMContentLoaded|document\.write|window\.lazyLoadOptions' );

Related ticket: https://secure.helpscout.net/conversation/1370346121/223946

Backlog Grooming (for WP Media dev team use only)

  • Reproduce the problem
  • Identify the root cause
  • Scope a solution
  • Estimate the effort
@Tabrisrp Tabrisrp added this to the 3.8.1 milestone Dec 17, 2020
@Tabrisrp Tabrisrp added 3rd party compatibility Issues related to 3rd party compatibility like theme, plugin or hosting module: file optimization needs: grooming type: bug Indicates an unexpected problem or unintended behavior labels Dec 17, 2020
@engahmeds3ed engahmeds3ed added GROOMING IN PROGRESS Use this label when the issue is currently being groomed. and removed needs: grooming labels Dec 17, 2020
@engahmeds3ed
Copy link
Contributor

engahmeds3ed commented Dec 17, 2020

Thanks @vmanthos

Reproduce the problem
I can see the error locally.

Identify the root cause
N2R is gobal function that is added inside our DOMContentLoaded event.

window.addEventListener('DOMContentLoaded', function() {
//N2R is defined here
});

so when calling it inside code that contains jQuery $, it's excluded from being enclosed by the previously mentioned DOMContentLoaded event.

N2R('documentReady',function($){

so it gives an error that N2R is not defined

Scope a solution
We need to exclude inline N.N2_ from being enclosed by DOMContentLoaded event.
and this could be done by one of two ways:-

  1. Use the filter rocket_defer_inline_exclusions inside thirdparty compatibility file.

  2. Adjust the code here

    $inline_exclusions = apply_filters( 'rocket_defer_inline_exclusions', 'DOMContentLoaded|document\.write|window\.lazyLoadOptions' );

and make new attribute called inline_exclusions which is an array and then when calling the filter we implode with | character to be added properly to the regex pattern.

@Tabrisrp I don't know if we need to call product team to take a decision on that or proceed with second approach here?

Estimate the effort
[XS]

@Tabrisrp
Copy link
Contributor

@engahmeds3ed You can go ahead with solution 2 👍🏼

@Tabrisrp Tabrisrp added effort: [XS] < 1 day of estimated development time and removed GROOMING IN PROGRESS Use this label when the issue is currently being groomed. labels Dec 17, 2020
@engahmeds3ed engahmeds3ed linked a pull request Dec 17, 2020 that will close this issue
@iCaspar iCaspar mentioned this issue Dec 21, 2020
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rd party compatibility Issues related to 3rd party compatibility like theme, plugin or hosting effort: [XS] < 1 day of estimated development time module: file optimization type: bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants