Sharing: revert changes to echo inline sharing js

[jeherve]

Fixes #6640
Reverts changes introduced in #6339

$service->display_footer() includes calls to js_dialog when not using Jetpack's Official sharing buttons.
js_dialog often uses wp_add_inline_script(), thus recreating the issues described in #6640.

Testing instructions:

1. Enable Sharing
2. Add a few non-official sharing buttons to your posts, like Facebook, Google+, and Twitter.
3. Open one of your posts.
4. When clicking on a sharing button, sharing windows should open in a small pop-up. Before this PR, and since Remove unused JS logic in email sharing #6339, they opened in a separate window since inline scripts for each button weren't outputted on the page.

Proposed changelog entry for your changes:

• Sharing: make sure that sharing buttons open in a small pop-up instead of a separate window.

cc @kasparsd who worked on the original PR.

[zinigor]

Fixes the problem, plus does not cause regressions WRT #6339. Thanks!

[kasparsd]

Before this PR, and since #6339, they opened in a separate window since inline scripts for each button weren't outputted on the page.

I'm not sure I follow this. #6339 only moved the inline scripts to a later wp_footer hook which is after all the JS has been printed. Moving it back before the jQuery is loaded will cause the same issue as before.

[zinigor]

OK, I forgot to test against the error that was caused by loading scripts in the footer, disregard my earlier review.

[jeherve]

Moving it back before the jQuery is loaded will cause the same issue as before.

That's correct. This PR is only reverting the changes introduced in #6339, since they stopped Jetpack from outputting inline sharing scripts in the footer.

[kasparsd]

@jeherve Looks like we're good, if we don't add back that unused jQuery call:

jQuery( document ).ready( function(){ document.getElementById('jetpack-source_f_name').value = '' });

[kasparsd]

And a permanent fix is to ensure that all display_footer() methods use wp_add_inline_script() instead of a direct JS output if they depend on jQuery being loaded.

[zinigor]

I have added wrapping of display_footer script output with wp_add_inline_script. Everything seems to be working fine with footer output of jQuery, but this turned into a quite heavy PR.

@kasparsd how do you find jQuery loading in the footer working for you? I'm having problems running my testing site like that because some themes try to use it in the header, notably TwentySeventeen.

@jeherve how would you feel about that change going into the point release?

cc @dereksmart @eliorivero for a second glance. You can find testing instructions in the description and comments to #6339

[kasparsd]

@zinigor Loading jQuery in the footer works fine now.

@zinigor How about refactoring the popup opener to properly escape the $name and $opts variables kasparsd@fb7847d

[eliorivero]

Works fine, 🐑

[zinigor]

This PR needs more work and more testing, we can't include all this in 4.7.1 sadly. We will work on this one in the following release cycle, plus test all these changes in WordPress.com. We'll also see that all the scripts are properly escaping variables, remove all unneeded ones and clean up existing.

Thanks for contributing to this @kasparsd , your input has been very helpful! I've taken a look at your work in your fork of Jetpack, feel free to create a PR against this repository, we'll take a look.

[kasparsd]

@zinigor Yeah, this does feel like a major undertaking considering the amount of changes required to get rid of the inline jQuery calls.

[jeherve]

Closing this PR for now.

@kasparsd It looks like you're making some good progress with kasparsd#1

Feel free to open a PR against automattic/jetpack when ready, we can continue from there!

Thanks!