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

getByPath multilanguage prefers name over language path #1550

Closed
timohausmann opened this issue Mar 28, 2022 · 10 comments
Closed

getByPath multilanguage prefers name over language path #1550

timohausmann opened this issue Mar 28, 2022 · 10 comments

Comments

@timohausmann
Copy link

timohausmann commented Mar 28, 2022

Short description of the issue

Hey, I found using getByPath on a multilanguage site returns the wrong page if the last requested "url segment" of language path overlaps with an existing name.

  • PageID 1034 Products (/products, /de/produkte)
  • PageID 1035 Produkte (/produkte, /de/angebote)

Expected behavior

Get page purely based on the path, /de/produkte/ returning Products.

Actual behavior

getByPath('/de/produkte', ...) will return the second page, because the last segment – the name – matches:
Querying for this also works: /xxyyz-non-existing-path/produkte

Optional: Screenshots/Links that demonstrate the issue

I discovered this when client renamed the /bio page for ONE language to /en/profile and I got a 404 (it found the /processwire/profile page).

Steps to reproduce the issue

  1. Have a multilanguage site where the root page has a second language path like /fr
  2. Create a Test Page at /test
  3. Rename the second language path of the Test Page to /fr/profile
  4. Calling $pages->getByPath('/fr/profile', ['useLanguages'=>true]); wont return the test page

Setup/Environment

  • ProcessWire version: 3.0.184
  • URL Segments are not active
@Toutouwai
Copy link

Toutouwai commented Mar 29, 2022

Cross-linking to #1116 because I think it's related.
It seems like getByPath() is over-confident when it finds a page of the right name at a different path and doesn't sufficiently allow that URL segments can mean that a different page match is intended.

@timohausmann
Copy link
Author

Oh yes It's very similar. I only ran into this with multilanguage paths.

I just tested the 'allowPartial' => false option because I saw this comment in the code but the problem persists (it seems to find no multilang paths at all with allowPartial false).

@ryancramerdesign
Copy link
Member

@timohausmann I can't seem to duplicate it here. Let me know if you can spot something I'm missing. I tested on 2 different multi-language installations, one large multi-language site, and one local default "test-languages" profile install.

My homepage has 3 active languages, depending on the profile I'm testing, but I'll use the first as an example which has, "/" (default), "/de/" and "/es/", using the core LanguageSupportPageNames.

I created a page named /test/ and have all languages checked "active" on the Setup tab. For the "de" language name, I entered "profile" (so that its URL would be /de/profile/).

I created a /test.php file with this:

<?php namespace ProcessWire;
include("./index.php");
$page = $pages->getByPath('/de/profile', [ 'useLanguages' => true ]);
echo $page->path;

I viewed domain.com/test.php in my browser and this is the result I get:

/test/

I also tried using some other page names that already exist (replacing the "profile" part), but still getting the same result. Is it possible you don't have the "active" checkbox checked for the "fr" language of your /test/ page?

@timohausmann
Copy link
Author

That's weird, I followed your steps on a fresh docker but cannot reproduce your result.

/something/profile always returns a NullPage for me, so does /fr/page, /de/module etc. What works is /profile! But it's ambiguous, when two pages use profile in different languages it's a NullPage again.

The part about products in the first post was not accurate, I tested more and with these pages, getByPath('/test', ...) also gives me a Nullpage.

  • Vita (/vita, /jp/test/)
  • Test (/test, /jp/something/);

image

@timohausmann
Copy link
Author

timohausmann commented Apr 8, 2022

I tested this again on a fresh windows+xampp install and Multi-Language profile (my other sites were blank) - same issue.

Here is a screen record of the setup. Changed de/uber to de/profile, pasted your snippet in a test.php, getByPath returns a NullPage for /de/profile.
https://youtu.be/Ni82ujNZdZ4

@ryancramerdesign
Copy link
Member

@timohausmann I work off the dev branch for GitHub issues, since it is the most up-to-date, and I just read your first message again and see you mentioned PW 3.0.184. The entire path-to-page system has been rewritten since that version. While getByPath() is a method independent of that system, I'm guessing the core version must be the difference.

@timohausmann
Copy link
Author

Oh, I can confirm it works with 3.0.197. Thanks, I should make a habbit out of using the dev branch.

ryancramerdesign added a commit to processwire/processwire that referenced this issue Apr 27, 2022
@ryancramerdesign
Copy link
Member

@timohausmann Actually, I started being able to duplicate this one today, after various combinations with the "profile" page name from the root of the web site in a multi-language environment, and 3+ pages having the name "profile", I would sometimes get the /processwire/profile/ page rather than the intended one. I've pushed an update in an attempt to fix this. This might be related to one that @Toutouwai found before and I previously had trouble duplicating.

@matjazpotocnik
Copy link
Collaborator

matjazpotocnik commented May 7, 2022

@timohausmann, can you please check if Ryan's fix works for you?

@timohausmann
Copy link
Author

I tested again on current dev branch and cannot reproduce the issue.

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

No branches or pull requests

4 participants