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

$sanitizer->pagePathName() inconsistent with $sanitizer->pageName() #1160

Closed
Toutouwai opened this issue Apr 25, 2020 · 3 comments
Closed

$sanitizer->pagePathName() inconsistent with $sanitizer->pageName() #1160

Toutouwai opened this issue Apr 25, 2020 · 3 comments

Comments

@Toutouwai
Copy link

Short description of the issue

The description in the documentation for $sanitizer->pagePathName() is minimal:

Returned path is not guaranteed to be valid or match a page, just sanitized.

This could do with fleshing out a bit about what kind of sanitization is done, but the documentation for $sanitizer->path() gives some more information that is relevant.

Path is validated per ProcessWire "name" convention of ascii only [-_./a-z0-9] As a result, this function is primarily useful for validating ProcessWire paths, and won't always work with paths outside ProcessWire.

This method validates only and does not sanitize. See $sanitizer->pagePathName() for a similar method that does sanitiation.

(Sidenote: typo above in "sanitiation")

I understand this to mean that $sanitizer->pagePathName() should sanitize path segments according to PW's page name convention. But it doesn't...

2020-04-25_164648

Setup/Environment

  • ProcessWire version: 3.0.153
@adrianbj
Copy link

I went through this again the other day until I looked at one of my modules that deals with this and discovered that the key is to use this:
image

Note that I've added ... to your example string which is connected to my issue posted the other day:
#1305

So this is also a problem, because using the Sanitizer::translate option actually does remove the ... so it's not even matching the behavior of the core when creating a page name from a title.

Maybe this doesn't seem like a big deal, but the other day I was importing content from an old site and I need to match page names (in PW) to the data from the old site - it's these sorts of situations where having consistency becomes so important.

@ryancramerdesign
Copy link
Member

@Toutouwai Thanks, I've refactored this method so that it is consistent with the pageName() sanitizer. Please let me know if you spot any other inconsistencies.

@matjazpotocnik
Copy link
Collaborator

@Toutouwai I've tested, and it looks like it works now. Can you double-check and close?

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