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

Sharing: Need to ensure jQuery loads earlier #4661

Closed
kraftbj opened this issue Aug 4, 2016 · 6 comments
Closed

Sharing: Need to ensure jQuery loads earlier #4661

kraftbj opened this issue Aug 4, 2016 · 6 comments
Assignees
Labels
[Feature] Sharing Post sharing, sharing buttons [Type] Bug When a feature is broken and / or not performing as intended
Milestone

Comments

@kraftbj
Copy link
Contributor

kraftbj commented Aug 4, 2016

To reproduce:

  • Use a theme like _s without any other plugins active. _s, out of the box, does not enqueue jQuery.
  • Run Sharing

Expected:
No JS errors in the console

Actual:
jQuery not defined error.

What's happening is we're enqueuing sharing.js in the footer (with jQuery as a dependency) here: https://github.com/Automattic/jetpack/blob/4.1.0/modules/sharedaddy/sharing-service.php#L536

Within that same function, we're then calling the display_footer for each enabled function:
https://github.com/Automattic/jetpack/blob/4.1.0/modules/sharedaddy/sharing-service.php#L548

display_footer is calling js_dialog for that service. Example: https://github.com/Automattic/jetpack/blob/4.1.0/modules/sharedaddy/sharing-sources.php#L727

js_dialog is printing JS straight to the OB, including the jQuery call: https://github.com/Automattic/jetpack/blob/4.1.0/modules/sharedaddy/sharing-sources.php#L331

This is all being outputted before the initial function closes, so WordPress enqueues sharing.js (and thus, right before it when it hasn't already been enqueued, jQuery) after we've printed the JS calling jQuery.

Original report: https://wordpress.org/support/topic/jquery-not-defined-10

@kraftbj kraftbj added [Feature] Sharing Post sharing, sharing buttons [Type] Bug When a feature is broken and / or not performing as intended labels Aug 4, 2016
@kraftbj kraftbj added this to the 4.2.1 milestone Aug 4, 2016
@Otto42
Copy link

Otto42 commented Aug 4, 2016

Suggestion: check for the wp_add_inline_script function (WP 4.5 and later) and switch to using that instead, so that you can have the output sure to happen after jQuery.

For earlier versions of WP, probably simply put that script output into its own action hooked to wp_footer, with a lower priority.

@jeherve
Copy link
Member

jeherve commented Aug 4, 2016

Related: #1223

@kraftbj
Copy link
Contributor Author

kraftbj commented Aug 4, 2016

Thanks @Otto42! 4.5 will be our minimum version supported once 4.6 ships, so that'll be the way to go.

@retrocausal
Copy link

retrocausal commented Apr 13, 2017

#6977 Nothing has been fixed here! I am sorry but that is the case.
Jetpack Sharedaddy is a Render Blocker, If it needs used.

screenshot from 2017-04-14 02-06-57
First of all, sharedaddy injecting inline scripts is not a correct way to add scripts. Can't it be loaded as an external resource? so it can be cached, and loads deferred?
If you HAD to inject them, why cant the module sharedaddy, add a very very very basic domcontent loaded check?
what if If jQuery is deferred?

Note:I have no experience developing with wordpress. Just an end user, curious to know, If Jetpack wants us to host sites with unchecked dependencies or wants us disregarding time to first paint by forcing jQuery and such Huge JS Libraries to Block Rendering basic content only because Jetpack needs libraries downloaded and parsed.
In a world where performance and milliseconds count for user experience, What tangible benifits would a render blocking plugin offer, that seems more essential?

@georgestephanis
Copy link
Member

@Nitin-Prabhakar I'm going to be picking up the discussion on the other issue then, as while the symptoms may be similar, the causes are different. Also, notifications can sometimes be wonky and easy to miss on closed issues.

@retrocausal
Copy link

Sure! Thanks Much for the attention it needs 👍
cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Sharing Post sharing, sharing buttons [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

No branches or pull requests

7 participants