Skip to content
This repository has been archived by the owner on Nov 6, 2019. It is now read-only.

camptix.js code not working because of timing condition. (tix-js, has-dynamic-js, wp_footer) #241

Open
EliW opened this issue Jul 16, 2019 · 2 comments

Comments

@EliW
Copy link
Contributor

EliW commented Jul 16, 2019

Hey team. So I recently noticed on a website that the display looked a little different. Specifically the "should the receipt go to this person" radio button text was appearing, which is supposed to be hidden if Javascript is working.

I started looking into it, and it looks like there's a bug in how things are being done, timing wise.

Basically the block of in camptix.js, starting at line 20 in the ($) block, isn't executing properly. This section:

/**
 * CampTix Javascript
 *
 * Hopefully runs during wp_footer.
 */
(function($){

And it's documented exactly why it isn't. That code, as designed, has to be run in the footer, because it expects that the entire page needs to be loaded first. The page isn't, and so basically all that code in that ($) block fails silently.

However, the script being attached in camptix.php, doesn't have the 'in footer' flag turned on. It looks like this starting on line 474:

		wp_register_script(
			'camptix',
			plugins_url( 'camptix.js', __FILE__ ),
			array( 'jquery' ),
			filemtime( __DIR__ . '/camptix.js' )
		);

There are two possible solutions. One is just to update that code to put 'true' in the footer field, ie:

		wp_register_script(
			'camptix',
			plugins_url( 'camptix.js', __FILE__ ),
			array( 'jquery' ),
			filemtime( __DIR__ . '/camptix.js' ),
			true
		);

The other (or in addition to) is to change the (function($) in the javascript, to be a proper document.ready ... so that it doesn't matter where it's included. It won't actually execute until the page is ready. IMO, this is the proper fix regardless of whether the code is included in the footer or header. Because then it doesn't matter.

I'm happy to submit a pull request fixing this. Just would like guidance as to the preferred fix (attach in footer, document.ready, both)

So that I don't submit a PR that gets rejected because the other one is preferred.

@EliW
Copy link
Contributor Author

EliW commented Jul 16, 2019

Tagging @coreymckrill again, just to make sure this gets seen since it's a real bug w/ consequences for folks at the moment, and a fix might be wanted soon.

@coreymckrill
Copy link
Contributor

@EliW thanks for pinging. This issue was discussed in our bug scrub chat today:
https://wordpress.slack.com/archives/C08M59V3P/p1566494342140000

Both changes seem worth doing, although we will need to test to make sure there aren't any negative side effects. I'm not sure why the in_footer flag isn't currently set to true. It could be a mistake, but there might also be a legitimate reason.

Are you still up for submitting a PR for this?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants