Tagged: cornerstone
-
AuthorPosts
-
August 28, 2016 at 1:24 pm #1150702
We were having the same issue as the woman in the thread below was having in relation to cornerstone and its integration with wpml:
https://community.theme.co/forums/topic/issue-with-wpml-errors/
The error stems from the x_share/share shortcode and cornerstone’s filter for the_title, as defined in cornerstone\includes\integrations\conflict-resolution.php on lines 14-20:
if ( class_exists( ‘SitePress’ ) && apply_filters( ‘cornerstone_enable_wpml_integration’, true ) ) {
add_filter( ‘pre_get_posts’, array( $this, ‘wpml_get_posts’ ) );
add_filter( ‘the_title’, array( $this, ‘wpml_title’ ), 99, 2 );
add_filter( ‘the_permalink’, array( $this, ‘wpml_permalink’ ) );
//Due to deep connection with cornerstone to the_excerpt, let’s not add another excerpt filter for now
//add_filter( ‘get_the_excerpt’, array( $this, ‘wpml_excerpt’) );
}In the other thread, you recommended that Danielle S. turn off error displaying rather than actually fixing the error. This is never a good idea.
So I’ve fixed it for you:
You are only passing in the post_title when applying the filter for the_title, when your function wpml_title requires both the page title and id of the post.
In cornerstone\includes\shortcodes\share.php, replace line 35 with the following:
$page_for_posts = get_page( get_option( ‘page_for_posts’ ) );
$share_title = ( $share_title != ” ) ? esc_attr( $share_title ) : urlencode( apply_filters( ‘the_title’, $page_for_posts->post_title, $page_for_posts->ID ) );The share shortcode seems the be the only place that particular filter is applied in either cornerstone or the x-theme files, so that should be sufficient to fix the error.
Please stop telling people to turn off error display instead of fixing your buggy code.
August 28, 2016 at 4:20 pm #1150779Hi there,
Thank you for the fix. We really appreciate your efforts 🙂
Cheers!
August 28, 2016 at 5:23 pm #1150826Another major issue with this is that you are running a filter on the_title on every single the_title() function call, even though this is literally only used for one shortcode. We hadn’t updated x-theme on our site since you had integrated cornerstone because our theme is so heavily modified. I finally went through the update on August 26, and saw a huge spike in the average call time for the_title because of the filter you have placed on it. I disabled the filter earlier today and the average call time dropped dramatically again. I feel bad for customers who do not have access to analytics tools such as new relic and are not aware of what effect each theme/plugin and their updates have on their site. Please see attached screenshot for graph.
August 28, 2016 at 8:20 pm #1151042Hi there,
It’s not an error, it’s just a warning. A warning can be triggered by anything even if it has no problem at all. And if you’ll check a freshly installed WordPress, you’ll notice that debugging is default to off. It’s WordPress intended configuration and not something we just decided. It just happens that on her setup, the WP_DEBUG = false is ignored hence displaying all warning and notices. So, we just suggested to force it too by turning off error display at once and directed them on error log file, we didn’t remove it. We only recommended that because WordPress can’t turn it off based on its default configuration.
If it’s a fatal error which usually causes blank pages, then we should get many complaints about it regardless of hosting provider. But I agree, the 2 missing arguments should be removed and we’re aware of that. Providing a temporary solution is all we could do as the support. But I already forwarded this for investigation.
The code is only responsible for retrieving the translation, changing your shortcode’s code will prevent it from the translation. And since it’s about translation which also share the same functionality from WPML, then even if it’s a simple the_title, it will still consume some time and process load as big as WPML can. WPML isn’t a simple plugin, that 205M is probably the load allocation for the whole object (WPML).
It can’t stay to 10mb forever as we add functionality over and over, it will increase over time especially if there are 3rd party features that are needed to make them possible. Even Revolution slider now requires 256mb as they provide updates. I’m not saying we’ll stop improving it, just explaining why it turns out like that.
And usually site’s are optimized by 3rd party plugin too (caching, transients, CDN, and more to prevent processing them over and over). Our theme and cornerstone can’t do it alone, it can’t limit WPML on how much memory should it use (maybe possible with Object cache).
Should we remove the translation from the title? Or shortcode titles should not be translated? Maybe we can think some workaround on that one instead of removing them.
Thanks!
August 28, 2016 at 10:14 pm #1151164No, don’t remove the translation from the title, but don’t attach such a resource-intensive filter to a wordpress function that is called at least once per page, and often multiple times per page, when you’re only using it for a shortcode that allows sharing content on fb/twitter/etc. Of course this only affects sites that also have WPML installed, but on our high-traffic site this one filter made a huge difference on our server load. I would rather not have to investigate every hook and filter after every theme/plugin update, but I guess that’s what I’m in for.
And, actually, now that I look at the WPML code, they are allowing you to get the correct title for the page by filtering the get_option(‘page_for_posts’) call that you make within the share shortcode. It returns the ID of the current language’s page for posts. You then use the ID to get the correct title. So yes, it’s still working on our site without the cornerstone wpml filter. Share within Spanish returns the Spanish title, and English returns English.
And I understand that the error wasn’t fatal, but it was an error, and it was a simple fix. Explaining to Danielle S. how to implement your temporary solution took longer than the actual solution.
August 28, 2016 at 10:51 pm #1151186Hi there,
I understand, will forward that. But not attaching to the filter is the same as removing it.
I understand that each user has their own way of optimization. I myself will use object caching/transients or likes to prevent the codes from re-executing on each load. That’s just another recommendation from my end.
And right, it will get the ID for page_for_posts, but not the ID of the current page. The translation isn’t just limited to page_for_posts. It could be right on your current setup, but I can’t say it’s wrong either. We’ll just be going to investigate this.
Danielle S.’s issue since is not related to the share shortcodes, she didn’t mention any particular details. Hence, we only provided a general and quick solution in case it appears on different areas too. It will take longer if we’ll going to add temporary fix one by one for each affected area. I’m not saying we’re not gonna fix it, we’re still investigating it. Your solution is correct and thanks for sharing, but it may be different from other users.
Thanks, we’ll look into this.
August 29, 2016 at 8:58 am #1151729There is only one filter for the_title in the entire x-theme/cornerstone codebase and it is declared within the Cornerstone_Integration_Conflict_Resolution class constructor. It is only being applied within the share shortcode in the code block that gets the title for the page_for_posts between lines 32 and 36. As your theme and plugin are written now, this solution applies for every user who relies on WPML along with x-theme and cornerstone.
Thankfully, New Relic allowed me to find the issue quickly by showing the dramatic spike in average call time on the_title or we would have been very frustrated.
August 29, 2016 at 9:12 am #1151768Thanks for the details.
-
AuthorPosts