Closed notabeatle closed 4 years ago
That's strange, I had noticed this issue yesterday while testing it on a few dev sites, but it should be resolved. I'm so far unable to recreate the issue, even with a fresh install using nLingual + WooCommerce + TwentyTwenty. So far the home URL isn't redirected to the page's normal name, nor does the home link break on other pages.
Can you give me an outline of what setup you're using?
Haha, we have a pretty extreme plugin count and a lot of custom code, so it could well be some bad interaction there. This time I didn't reproduce with a fresh installation like I did the last time I bugged you, so it may just be our environment. I did confirm that it doesn't happen without nLingual enabled, and that it doesn't happen with 2.8.10. We are still on 5.4, not 5.5, [edit: of Wordpress, that is] if that matters or is known to cause problems, though I bumped to latest WooCommerce on our testing instance to see if that was the culprit (it wasn't).
If I get a chance today I'll try to reproduce the issue on a fresh, blank 5.4 installation, and if the behavior crops up there I'll try going to 5.5 and see if that resolves it. If it doesn't show up in either case then I've got some digging to do, I guess. Thanks for checking it out.
I don't think 5.5 has anything to do with it, though I'm wondering if it has something to do with the change I made to Rewriter::localize_here()
where it first checks if it's the homepage, then checks if it's some kind of post/page/etc.
Regarding issue 2, it sounds like the link is in fact printing an empty string; can you confirm that the actual markup is printing the current page link.
My next guess is that something is causing page_on_front to return 0/null and so get_permalink is simply using the current post's URL. I'll do some digging to see what could've changed that affects that.
OK, here's what I just tried:
/
?page_id=5&nl_language=en
These show up as 302 redirects (from /
) in my logs.
Then I tried setting URLs for the default language will be unmodified.
, with the result that the redirect dropped the nl_language
part but retained the rest (/?page_id=5
)
Then, enabled proper permalinks in WP settings. Redirect still happens, but now to /test-homepage/
, as seen on our site.
Disable nLingual, behavior reverts to default (no redirect).
Now here's something odd:
/sample-page
and /
respectively./sample-page
is unchanged, but Test Homepage now links to /test-homepage
This is all under theme Twenty Twenty (the default).
So it does seem to be happening with vanilla WP, unless I've done something wrong here.
HOWEVER I was not able to find a way to reproduce links intending to point at the homepage linking instead to the current page. I haven't dug far enough into Twenty Twenty's header code to tell how they're generating those links, yet.
Okay that's just maddeningly strange. I'll do somet further testing tonight to see if I can recreate it.
Just tried WP 5.5, same behavior. Under PHP 7.2 in both cases. Just bumped up to 7.4.9 just to cover all my bases. Same behavior.
version: '3.1'
# Default mysql user: root
services:
db:
image: mysql
command: --default-authentication-plugin=mysql_native_password
restart: always
environment:
MYSQL_ROOT_PASSWORD: nlingual
ports:
- 3306:3306
adminer:
image: adminer
restart: always
ports:
- 8091:8080
wordpress:
image: wordpress:5.5.0-php7.4-apache
depends_on:
- db
restart: always
environment:
WORDPRESS_DB_USER: root
WORDPRESS_DB_PASSWORD: nlingual
WORDPRESS_DB_HOST: db
WORDPRESS_DEBUG: 1
volumes:
- ./plugins:/var/www/html/wp-content/plugins:rw,z
ports:
- 8090:80
Quick & dirty docker-compose.yml I've been using to test, in case it's useful. You have to go through WP setup on first run, but as long as you don't rm the database container it shouldn't make you do that again. Just dropping nlingual in that mapped ./plugins
directory straight from git, as in ./plugins/nlingual
, so there's no need to install it after WP is up, should already be there if you put the plugin in place ahead of time.
The :rw,z
permissions mods on the end of that directory mapping may not be necessary on macOS, but were on Linux. No clue how to make it work if you're on Windows, as I've never used Docker there.
WordPress is served on port 8090 with this file, unless you change it.
Okay, got a docker instance spun up (holy crap I need to start using this more), and was able to recreate the issue.
Digging into the cause now but as far as the nl_language=en
bit that's technically intended, due to the default permalink "structure" being via GET params, and the default handling being to localize URLs for the default language. It's not an outright issue but I'm debating changing some of the defaults in the next release.
Right, I get that nl_language=en
is working as intended, and that part's fine. Just included observations about its behavior under different settings for completeness. The way it dropped off the redirect to page #5 with default permalink structure maps just fine to how it works with page-slug redirects, I think, and it was (correctly) absent from default lang permalinks when default language redirects were disabled.
Let me know if I can do anything else to help out. Thanks a bunch for looking at this.
Okay I found the culprit: https://github.com/dougwollison/nlingual/blob/cea14b12abc26b52340549f256aa108799846f42/includes/class-nlingual-frontend.php#L468
When I rewrote the current_language_post
hook to check and make sure it wasn't returning an unpublished post, I didn't include logic to handle get_post_translation
returning false (for un-assigned/translated posts). This caused is_front_page()
to return false because it's comparing page_on_front
(which was rewritten to false/0) to the current page. Technically this didn't break anything until WP_Query finishes setup and sets the $post
global, which means any later attempts to rewrite page_on_front
cause it to compare the queried post (the real homepage) to itself (the default post when passing null/false/0 to a function like get_post_status
) rather than a non-existent one.
It's fixed now, as for the other issue with the homepage links pointing to the current page, not sure what could be happening there but might be tangentally related to page_on_front
getting set to 0.
Just tried out master
on our testing site. All issues appear to be resolved, including the weirdly-broken home links.
Quickly kicked the tires on client language preference redirection and that still seems to be good, no regression.
Awesome. Thanks again.
Sweet. Thanks for all the detailed info, really appreciate it.
Version: 2.9.0
Situation: Site uses a page as its homepage (the URL of which is typically retrievable with
get_option('page_on_front')
). This happens to be named "desktop", in this case. Without nLingual, and under nLingual 2.8.10, this resolves simply toexample.com/
with no page slug. Under nLingual, the home page is redirected toexample.com/desktop/
, andget_option('page_on_front')
not only returns thatdesktop/
path while on the homepage, but proceeds to return whatever the current page is, as one nagivates around the site, such that a link that would normally always point at the homepage (simply,example.com/
) points instead to, say,example.com/some-page
while visitingsome-page
.So there seem to be two things going on, that are different from behavior under vanilla WP or nLingual 2.8.10:
example.com
, end up onexample.com/desktop
)page_on_front
option, and ends up providing the current page path for some reason.The first is just undesirable, but the second actually breaks things.
Possibly relatedly, I'm also seeing page URL lookups built into WooCommerce, for things like the cart or checkout URLs, return a link to the site root instead, with the result that these links are non-functional with nLingual 2.9.0 enabled. There seems generally to be something odd happening with internal WordPress page URL lookups, though not with lookups for the purposes of request routing, as far as I can tell (if you put the URLs in manually, they work).
(It may be premature to report bugs in 2.9.0 since it's not up on the plugin market yet, but given the version bump on master I thought it safe to at least try out; if you already knew or suspected it has issues of this sort and aren't ready for anyone to be messing with it, I'll take no offense at a fast close on this issue)