Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ACF Post Object [qTranslate-XT] does not work #1320

Closed
salt-istanbul opened this issue Apr 12, 2023 · 7 comments
Closed

ACF Post Object [qTranslate-XT] does not work #1320

salt-istanbul opened this issue Apr 12, 2023 · 7 comments
Labels
bug Something isn't working, reproducible module: ACF Integration with ACF severity: major Major functionality

Comments

@salt-istanbul
Copy link

ACF Post Object [qTranslate-XT] field always getting 400 Bad Request. I can't get any post objects.
I checked ACF's native Post Object field but it's working.

ACF Pro 6.1.3
qTranslate-XT 3.14.0

@salt-istanbul salt-istanbul added the bug Something isn't working, reproducible label Apr 12, 2023
@herrvigg
Copy link
Collaborator

I confirm there's a bug, I can reproduce it. The standard ACF Post Object is working, without translation.

@herrvigg herrvigg added severity: major Major functionality module: ACF Integration with ACF labels Apr 12, 2023
@herrvigg
Copy link
Collaborator

Indeed there's a regression in 3.14.0. Now fixed in master. You can also copy the 4 lines of code shown in the patch above: #2e8e453.

@salt-istanbul
Copy link
Author

Indeed there's a regression in 3.14.0. Now fixed in master. You can also copy the 4 lines of code shown in the patch above: #2e8e453.

thanks, ACF Post Object [qTranslate-XT] is working now but I'm getting errors similar to the following after your latest release.
Fatal error: Uncaught TypeError: qtranxf_checkCanonical(): Argument #2 ($requested_url) must be of type string,
I'm not sure but this problem about function argument's type declarations.

@herrvigg
Copy link
Collaborator

What is following "must be of type string, ..."?
By case, do you use Yoast (wp-seo)?
I see there are two potential problems (1) this function is used wrongly by a hook in the Yoast module, (2) the WordPress documentation is not reliable, they say string are passed but it can be other types 😕

@salt-istanbul
Copy link
Author

What is following "must be of type string, ..."? By case, do you use Yoast (wp-seo)? I see there are two potential problems (1) this function is used wrongly by a hook in the Yoast module, (2) the WordPress documentation is not reliable, they say string are passed but it can be other types 😕

Yes, i'm using Yoast(wp-seo) and i think this function is used wrongly by a hook in the Yoast module.
Because $requested_url atgument is return "undefined" on qtranxf_checkCanonical function.
There's already a comment before this function line "// TODO check API with Yoast, seems it's only 1 parameter?"

Same problem on qtranxf_wpseo_webpage_schema( array $piece, string $context ) function.
qtranslate-xt\src\modules\wp-seo\wp-seo-front.php on line 34

$context atgument is return "undefined"and getting this error.
Fatal error: Uncaught TypeError: qtranxf_wpseo_webpage_schema(): Argument #2 ($context) must be of type string,

I'm getting no error when I remove this arguments from functions.

herrvigg added a commit that referenced this issue Apr 15, 2023
The main purpose is to filter `redirect_canonical` (WP hook).
The WP documentation is not precise regarding the types so we have
to relax the types check. Check first if the redirect URL is false.

The second parameter is the requested URL but it is ignored.
See #1320, wrong usage with Yoast. The second parameter has a
different meaning with Yoast. Removing the types will work but the
Yoast hook should be rewritten with a different function.

Rewrite the function to handle the default language in convertURL.
herrvigg added a commit that referenced this issue Apr 15, 2023
See also #1320. It is too risky to specify these types for now.
@herrvigg
Copy link
Collaborator

Yes, the Yoast hooks are ill-defined. The second parameter in Yoast is Indexable_Presentation, not a string giving the requested URL that is expected in qtranxf_checkCanonical. Though this parameter is not used, I reverted the type checks so it should work now. I also reverted all the type checks in the Yoast hooks, it feels too risky at this point. Please retry with master, ACF and Yoast should be fixed for you now.

Strong type checks with PHP is the way to go because it helps to find such wrong usage. The drawback is that is it can create regressions. It's unfortunate the WP documentation is not accurate and their APIs are so messy with many mixed types often undocumented. I will comment more in #1314 and possibly do other reverts.

@herrvigg
Copy link
Collaborator

Released fix for ACF Post Object in 3.14.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working, reproducible module: ACF Integration with ACF severity: major Major functionality
Projects
None yet
Development

No branches or pull requests

2 participants