Comments (30)
Comment by johnclause
Thursday Feb 12, 2015 at 00:08 GMT
On second thought, if you really have a different solution, which is tested and ready to go, submit it with pull request, and we will go from there?
from qtranslate-xt.
Comment by johnclause
Thursday Feb 12, 2015 at 03:26 GMT
What if we add these two lines at the beginning of the function:
if( !isset($clauses['join']) ) return $clauses;
if( strpos($clauses['join'], $wpdb->posts ) === FALSE) return $clauses;
Did your error messages disappear? Is that good enough for you? If you need to still filter them, then try this:
if( !isset($clauses['join']) || empty($clauses['join']) ){
$clauses['join'] = "JOIN $wpdb->posts ON $wpdb->posts.ID = $wpdb->comments.comment_post_ID";
}elseif( strpos($clauses['join'], $wpdb->posts) === FALSE ){
//do not break some more complex JOIN
return $clauses;
}
from qtranslate-xt.
Comment by johnclause
Thursday Feb 12, 2015 at 03:34 GMT
Please, also tell me if option "Hide Untranslated Content" makes sense to you at all? I personally do not use it, since I think people should decide themselves whether they want to read untranslated to their language post. They might choose to translate it themselves, but when it is hidden, then they do not know that it existed.
from qtranslate-xt.
Comment by side777
Thursday Feb 12, 2015 at 08:35 GMT
Option "Hide Untranslated Content" perfectly makes sense. I run a couple of sites where i have some posts that are just not relevant for the other languages. plus: it's a seo issue - 'duplicate content'...
regarding the 'people should decide themselves' thing: i run a different approach - people who are logged in can choose which languages they understand in the profile settings and see posts of all languages - the main lang (the one they choose in the browser / the url) always first.
the "Hide Untranslated Content" option is especially effective for 'latest posts' widgets. if your last 3 posts are untranslated you could easily drive people away from your site showing foreign headlines in suggested posts, etc.
from qtranslate-xt.
Comment by johnclause
Thursday Feb 12, 2015 at 08:38 GMT
Ok, point taken. Other people seem to also want it. What do we do about the code?
from qtranslate-xt.
Comment by side777
Thursday Feb 12, 2015 at 08:41 GMT
i see a big drawback in the current approach of filtering for post_content: it just doesn't tell you anything about the actual language of a certain comment if the corresponding post is translated or not. i have multi-lingual posts, people tend to comment in the language they are reading. so the filter hides comments for untranslated posts but still shows comments in a different language.
so it makes much more sense to add a lang meta to every comment when the comment is saved. the language would be the current user language. very easy. i'll provide code asap.
from qtranslate-xt.
Comment by johnclause
Thursday Feb 12, 2015 at 08:55 GMT
We want to hide the comments for the untranslated posts, not the untranslated comments. We do not care about the language of comment, it can be anything, we just want consistency, if post is hidden, then all comments to it also hidden regardless to their language.
If we also want to filter comments by specific language, then it would be a new and different option.
Could you answer the question, if the code I cited makes your problem go away? Did you figure out under which condition a query without JOIN to posts table happens?
I would much rather to do optimization later, because I plan to re-design the whole backend to be sane, as it is insanely inefficient now. I do not wish to do it in pieces, just for comments now.
Please, answer the question if the code above makes your site work. Do you need filtering for the case when the problem happens. Would be also good if you tell me finally when and how it happens.
from qtranslate-xt.
Comment by side777
Thursday Feb 12, 2015 at 10:19 GMT
i used the second snippet and it works.
from qtranslate-xt.
Comment by side777
Thursday Feb 12, 2015 at 10:45 GMT
what i found out until now: the get_comments(); function by default doesn't JOIN posts. if you query comment meta it adds INNER JOIN wp_commentmeta and only if you query post related stuff (like post_status, post_type, etc.) it adds JOIN wp_posts.
so a save join string would be
"JOIN $wpdb->posts ON $wpdb->posts.ID = $wpdb->comments.comment_post_ID INNER JOIN $wpdb->commentmeta ON ( $wpdb->comments.comment_ID = $wpdb->commentmeta.comment_id )"
you could test for both cases and only skip if the join string is more complex...
from qtranslate-xt.
Comment by side777
Thursday Feb 12, 2015 at 14:35 GMT
checking the core for 'WP_Comment_Query' (https://core.trac.wordpress.org/browser/tags/4.1/src/wp-includes/comment.php) led me to another major flaw in the current filter. as you can see in line 430 a CACHED version of the query is returned - ignoring any altered clauses! therefore dynamically altering the clauses leads to wrong returns, i.e. wrong languages displayed. especially if users run object caches. i tested this in my 2015 setup. i had to clear the cache to see the correct comments.
there are 2 safe possible solutions:
- altering the query vars ('pre_get_comments')
- filter the 'comments_array'
if it's even worse, the same problem applies to the posts! still checking...
from qtranslate-xt.
Comment by side777
Thursday Feb 12, 2015 at 15:21 GMT
it's true. post queries get also cached. so the 'hide untranslated' option is badly broken because it shows the wrong languages...
from qtranslate-xt.
Comment by johnclause
Thursday Feb 12, 2015 at 16:55 GMT
Why don't it get cached correctly at first time?
from qtranslate-xt.
Comment by johnclause
Thursday Feb 12, 2015 at 16:58 GMT
I thought this cache is for one session only, different users do not affect each other, do they?
from qtranslate-xt.
Comment by johnclause
Thursday Feb 12, 2015 at 17:05 GMT
You had to clear cache, because you were doing testing and changed the query in between the calls on the same session, is that correct? This should not happen at live site, will it?
from qtranslate-xt.
Comment by johnclause
Thursday Feb 12, 2015 at 17:24 GMT
From the page http://codex.wordpress.org/Class_Reference/WP_Object_Cache:
By default, the object cache is non-persistent. This means that data stored in the cache resides in memory only and only for the duration of the request. Cached data will not be stored persistently across page loads unless you install a persistent caching plugin.
from qtranslate-xt.
Comment by johnclause
Thursday Feb 12, 2015 at 17:29 GMT
Does it happen in your theme, that this query changes language during the same request? Which theme do you use, btw? If this is what you really need, then yes, we can set language in query_vars['lang']
via do_action_ref_array( 'pre_get_comments', array( &$this ) );
. The value query_vars['lang']
will not be used by query, but it will change the cache key.
from qtranslate-xt.
Comment by side777
Thursday Feb 12, 2015 at 20:33 GMT
read the source: it generates a md5 cache_key from the DEFAULT $args. any altering of the clauses happens later and is ignored by the function because the cache key is the same not matter what you do with the clauses later. so it returns the new request from the cache assuming that queryvars not changed == output the same.
with a caching plugin the cache is persistent. that means the next user gets YOUR output. wp generates cache for every different set of $args.
so this clauses altering breaks queries with ANY theme. try it with 2015! (i did.)
you are not going to tell the people they can't use caching plugins to hide untranslated posts, right? ;)
from qtranslate-xt.
Comment by side777
Thursday Feb 12, 2015 at 20:38 GMT
i wrote an alternative approach for posts filtering today. i set up an action to preg match the languages in use from the title of the current post in the loop and set a post meta IF the meta is not already there. (for backwards comp)
and i set up an action in save_post to update the meta not matter what.
then i alter the query with 'pre_get_posts'.
from qtranslate-xt.
Comment by johnclause
Thursday Feb 12, 2015 at 22:03 GMT
md5 cache_key from the DEFAULT $args
I understand, but default arguments uniquely defines what is done later, except us adding a fork by language. If we add query_vars['lang']
, the cache key will become different enough for the purpose.
with a caching plugin
Which one do you mean? Anyway, what I described with query_vars['lang']
should solve the problem even with persistent cache.
I am very glad you that you have better ideas and wish to implement them. I will be waiting for your pull request here.
Meanwhile I will put in query_vars['lang']
to be done for the moment, so I can release 3.0 now.
from qtranslate-xt.
Comment by johnclause
Thursday Feb 12, 2015 at 22:05 GMT
BTW, you never let me know which theme do you use? I am still curious.
from qtranslate-xt.
Comment by johnclause
Friday Feb 13, 2015 at 09:29 GMT
2.9.8.9 has your changes. Persistent cache did not work, because they cut all extra keys from query_vars
. Could you, please, test if it is ok for you without persistent cache? We will work on persistent cache later.
from qtranslate-xt.
Comment by side777
Friday Feb 13, 2015 at 09:41 GMT
so at least have a look at my working solution.. you get in lots of trouble if you break caching. people tend to not read any warnings and complain in the support forums later. believe me, i know exactly what i'm talking about...
if you insist on using a hacky workaround for your 3.0 that you could use any query var from the default list (in the core code, linked above) - i guess a meta value would do the trick. as mentioned before, this would be very hacky for a major release...
from qtranslate-xt.
Comment by side777
Friday Feb 13, 2015 at 09:42 GMT
i have never worked with github - as soon as i find out how pulling a request works and where to do it, i'll send you my solution.
from qtranslate-xt.
Comment by side777
Friday Feb 13, 2015 at 09:44 GMT
about my theme: i use a selfmade framework with child themes for the sites under my direct control. other sites i admin use several themes...
for testing i use a clean twenty fifteen setup.
from qtranslate-xt.
Comment by johnclause
Friday Feb 13, 2015 at 18:54 GMT
any query var from the default list
I did look into it right after the discovery, but there is no one we can change safely as far as I can tell.
Which caching plugin do you use? I can see a lot of problems with persistent cache, which such plugin must deal with. What if one user submits new comment? It is not going to show up for anyone until cache is invalidated? It can get extremely complex. I would say that if caching plugin does not work in this case then it is a problem of caching plugin. Let them figure out a solution. Many other plugins can alter query result like we do, and I am sure they do not think about persistent cache either. Caching plugin must have its own ways around.
Did you try the latest version? Does it work for you except persistent caching? Press Download button at plugin page: https://github.com/qTranslate-Team/qtranslate-x, which runs this link https://github.com/qTranslate-Team/qtranslate-x/archive/master.zip
GitHub has become one of the most popular standard for doing shared development. You will have to get engaged with them in any case for many different reasons, if you are in this line of business. Fortunately, they have pretty good documentation. Shortly: you fork the repository, clone it to your local computer, modify the code, push modifications to your fork, then from your fork it is two clicks to submit pull request here. You will find documentation on each step.
Please, try the latest version before anything else ;)
from qtranslate-xt.
Comment by side777
Saturday Feb 14, 2015 at 18:50 GMT
any query var from the default list
add this to a meta query:
array( 'key' => 'qtranxf_language_' . qtranxf_getLanguage(), 'compare' => 'NOT EXISTS' );
from qtranslate-xt.
Comment by side777
Saturday Feb 14, 2015 at 19:03 GMT
i use w3tc, which is very common. afaik most caching plugins make the wp cache persitent, btw.
What if one user submits new comment? It is not going to show up for anyone until cache is invalidated?
exactly. that's why the core invalidates the cache on any new comment. read the code.
it is a problem of caching plugin.
definitely not. i'd say it's a flaw in the core because it doesn't check for altered clauses. the caching plugin has no chance at all to handle this because it simply builds up upon the core cache function.
Many other plugins can alter query result like we do
wrong - i guess this approach is pretty specific: dynamically alter the SQL for not logged in users and for the exact same URL? i can't think of one plugin that alters the SQL to output different content at the same URL.
from qtranslate-xt.
Comment by johnclause
Friday Feb 20, 2015 at 21:59 GMT
I am sorry, @side777, about the delay in response. I got busy now with other things. Could you meanwhile explain the logic behind the following:
add this to a meta query:
array( 'key' => 'qtranxf_language_' . qtranxf_getLanguage(), 'compare' => 'NOT EXISTS' );
from qtranslate-xt.
Comment by side777
Monday Feb 23, 2015 at 13:50 GMT
^^ this makes the query individual for every language and different cache data is saved.
in a better future version you would save the languages used in a post in that meta key, compare 'LIKE' and skip the dirty manipulation of the query string, i.e. increase the performance of the query, PLUS avoid the cache problem.
from qtranslate-xt.
Comment by johnclause
Monday Feb 23, 2015 at 19:31 GMT
I understand that it makes key unique and helps caching, I am curious how does this affects the final query and why it is safe to implement. I can look it up, but I thought you would explained it to me since you already did the investigation.
from qtranslate-xt.
Related Issues (20)
- [Slug translation module] When category base is removed, it gives category 404 in other languages. HOT 3
- rank math pro (integration)
- [Slugs translation] qtranxf_convertURL doesnt translate slug translation urls HOT 2
- Slug module - %product_cat% in URL HOT 2
- If use pre-path mode, if the url applied parameters, when the page load, the parameter will lost.
- The content in the text editor is replaced by another language HOT 1
- Conflict 'Language Menu' & 'Woocommerce cart' nav items
- The category menu item disappears HOT 2
- Error with Slug active HOT 3
- Crash with classic editor? HOT 1
- 404 on REST API on adding default language prefix to url HOT 2
- Firstpage redirects 301 HOT 3
- Fatal Error on the Woocommerce Shop Page when using product filters HOT 4
- crash with Classic editor HOT 1
- excerpt field not showing translation even though it exists in the DB (since upgrade to 6.1)
- Integration in YOAST SEO problem
- Qtranslate Language Switcher Stuck - Chrome HOT 4
- Can't use wp-admin due to a typo in code HOT 1
- Language switcher not available on ACF Option Page HOT 3
- [Slugs] Page titles do not translate HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from qtranslate-xt.