Remove unused JS logic in email sharing by kasparsd · Pull Request #6339 · Automattic/jetpack

kasparsd · 2017-02-09T09:35:28Z

Fixes #1223

Changes proposed in this Pull Request:

Remove the JS that was previously removing a dummy input field value for spam protection.
Print inline scripts that depend on jQuery after WP core is done with print_footer_scripts(). We are not using wp_add_inline_script() because all the display_footer() methods echo instead of return their output.

Testing instructions:

Share a post via email. Ensure that the email was sent.

Fixes #1223

Ensures that dependencies are met.

grappler

Thank you @kasparsd for created the PR. The changes make sense.

zinigor

Looks good, sharing via email works fine, no more JavaScript error when enqueueing jQuery in the footer.

zinigor · 2017-02-10T12:47:14Z

To test you need to make jQuery load in the footer, it can be done using a filter like this:

function wpse_173601_enqueue_scripts() {
    $wp_scripts = wp_scripts();
    $wp_scripts->add_data( 'jquery', 'group', 1 );
    $wp_scripts->add_data( 'jquery-core', 'group', 1 );
    $wp_scripts->add_data( 'jquery-migrate', 'group', 1 );
}
add_action( 'wp_enqueue_scripts', 'wpse_173601_enqueue_scripts', 20 );

Keep in mind that in order for jQuery to remain in the footer, it has to have no dependent scripts loading in the header. One such script is the related posts JS, which you may have to disable or enqueue in the footer as well.

@see

* Changelog: update stable tag and move changelog to changelog.txt Also remove old releases from readme.txt to keep the changelog tab short. * Changelog: add #5883 Also update the filter's docblock to match new version. * Changelog: add #5938 * Changelog: add #6298 * Changelog: add #3405 * Changelog: add #5941 * Changelog: add #6239 * Changelog: add #6281 * Changelog: add #6303 * Changelog: add #6018 * Changelog: add #6300 * Changelog: add #6296 * Changelog: add #6130 * Changelog: add #6292 * Readme: remove extra "on". * Changelog: add #6307 * Changelog: add #3297 * Changelog: add #6275 * Changelog: add #6321 * Changelog: add #6297 * Readme: update the support forum link anchor. Anchor changed when WordPress.org forums were updated to bbPress 2 * Readme: update list of a12s, it wasn't up to date anymore! * Changelog: add #6338 * Changelog: add #6337 * Changelog: add #6335 * Changelog: add #6333 * Testing List: first version of the 4.7 testing list. * Changelog: add #6332 * Changelog: add #6325 * Changelog: add #6326 * Changelog: add #6339 * Changelog: add #6342 * Changelog: add #6343 * Changelog: add #6346 * Changelog: add #6347 * Changelog: add #6279 * Changelog: add #6306 * Changelog: add #6312 * Changelog: add #6316 * Changelog: add #6171 * Changelog: add #6317 * Changelog: add #6246 * Changelog: add #6263 * Changelog: add #4220 * Changelog: add #5888 * Changelog: add #3406 * Changelog: add #3637 * Changelog: add #6320 * Changelog: add #5992 * Changelog: add #6322 * Changelog: add #6324 * Changelog: add #6352 * Changelog: add #6355 * Changelog: add #6360 * Changelog: add #6362 * Changelog: add #6369, #6382 * Changelog: add #6370 * Changelog: add #6375 * Changelog: add #6383 * Changelog: add #6384 * Changelog: add #6386 * Changelog: add #6395 * Changelog: add #6403 * Changelog: add #6406 * Changelog: add #6418 * Changelog: add #6419 * Changelog: add #6434 * Changelog: add #6446 * Changelog: add #6006 * Changelog: add #6096 * Changelog: add #6399 * Changelog: fix typo. @see #6331 (comment) * Changelog: add #6440 * Changelog: add #6443 * Changelog: add #6445 * Changelog: add #6463 * Changelog: add #6468 * Changelog: add #6471 * Changelog: add #6474 * Changelog: add #6480 * Changelog: add #6497 * Changelog: add #6499 * Changelog: add #6514 * Changelog: add #6267 * Changelog: add #5940 * Changelog: add #6492 * Changelog: add #5281 * Changelog: add #6327 * Changelog: add #6451 * Changelog: add #6525 * Changelog: add #6530

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.

Reverted part of the changes introduced in #6339.

This JS was previously removed in #6339, but then added back (I think during a rebase) in #6332.

Remove unused JS logic

067bbc2

Fixes #1223

matticbot added the Touches WP.com Files label Feb 9, 2017

Print inline scripts after WP is done with footer scripts

93d8df8

Ensures that dependencies are met.

jeherve added [Feature] Sharing Post sharing, sharing buttons [Pri] High [Status] Needs Review This PR is ready for review. Bug When a feature is broken and / or not performing as intended labels Feb 9, 2017

jeherve mentioned this pull request Feb 9, 2017

Calling sharing_add_footer after (!) wp_print_footer_scripts in wp_footer to ensure jQuery dependancy #1223

Open

jeherve added [Pri] Normal and removed [Pri] High labels Feb 9, 2017

grappler approved these changes Feb 9, 2017

View reviewed changes

zinigor approved these changes Feb 10, 2017

View reviewed changes

zinigor added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Feb 10, 2017

dereksmart merged commit bba03ce into Automattic:master Feb 10, 2017

dereksmart added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Feb 10, 2017

jeherve added the [Status] Needs Testing We need to add this change to the testing call for this month's release label Feb 10, 2017

jeherve added [Status] Has Changelog and removed [Status] Needs Changelog labels Feb 20, 2017

jeherve added a commit that referenced this pull request Feb 21, 2017

Changelog: add #6339

96f5d74

jeherve mentioned this pull request Mar 13, 2017

Sharing: revert changes to echo inline sharing js #6641

Closed

zinigor added a commit that referenced this pull request Mar 14, 2017

Reverted part of the changes introduced in #6339.

b1a27a2

eliorivero added a commit that referenced this pull request Mar 14, 2017

Merge pull request #6649 from Automattic/fix/revert-sharing-6339

e3c8abe

Reverted part of the changes introduced in #6339.

eliorivero mentioned this pull request Mar 14, 2017

Reverted part of the changes introduced in #6339. #6649

Merged

dereksmart pushed a commit that referenced this pull request Mar 14, 2017

Reverted part of the changes introduced in #6339.

c813ec2

jeherve removed the [Status] Needs Testing We need to add this change to the testing call for this month's release label May 23, 2017

jeherve added a commit that referenced this pull request Nov 13, 2017

Sharing: remove inline JS used in the hidden email sharing form.

948c7da

This JS was previously removed in #6339, but then added back (I think during a rebase) in #6332.

jeherve mentioned this pull request Nov 13, 2017

Sharing: remove inline JS used in the hidden email sharing form. #8149

Merged

oskosk pushed a commit that referenced this pull request Nov 23, 2017

Sharing: remove inline JS used in the hidden email sharing form. (#8149)

40e9a33

This JS was previously removed in #6339, but then added back (I think during a rebase) in #6332.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove unused JS logic in email sharing#6339

Remove unused JS logic in email sharing#6339
dereksmart merged 2 commits intoAutomattic:masterfrom
kasparsd:bug/1223-jquery-email-sharing

kasparsd commented Feb 9, 2017 •

edited

Loading

Uh oh!

grappler left a comment

Uh oh!

zinigor left a comment

Uh oh!

zinigor commented Feb 10, 2017 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

kasparsd commented Feb 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes proposed in this Pull Request:

Testing instructions:

Uh oh!

grappler left a comment

Choose a reason for hiding this comment

Uh oh!

zinigor left a comment

Choose a reason for hiding this comment

Uh oh!

zinigor commented Feb 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

kasparsd commented Feb 9, 2017 •

edited

Loading

zinigor commented Feb 10, 2017 •

edited

Loading