Page Reference Field selector string issues

[fijha]

Short description of the issue

1. When using quotes inside Selector string with input field type Page Auto Complete, the admin page using this field break because its not properly escaped inside HTML data-url attribute.

2. Another issue is that checking if field is empty (field='' or field=) doesn't work either.

Suggestion for a possible fix

InputfieldPageAutocomplete.module (line 198):

"data-url='" . $this->wire('sanitizer')->entities($url) . "' " .

But checking if field is empty (field='' or field=) still doesn't work…

Setup/Environment

• ProcessWire version: 3.0.98

[ryancramerdesign]

Thanks. For item 1, I've added your suggested fix. For item 2, I think this is because ProcessPageSearch is what's generating the results, and it may require a little bit different syntax in this case, or we may need to update that module to support something here. What type of field are you trying to match? One thing you can try is field=0, which allows it to still have a value with length, and it might work in this case. If it's a multi-value type, then a field.count=0 may work.

[fijha]

Thanks. It looks like field=0 doesn't work with date field. I tried date=0 alone, but actually what I need to match is (date=''), (date>=today). But I think there is another problem with OR selectors. For example this won't work either (date>today), (date=today).

Do you think it's possible to use hook to filter the results somehow? That would by the best.

[ryancramerdesign]

This all passes through user input to ProcessPageSearch, so there are some limitations on the selector. It's rarely an issue because most use autocomplete for regular text searches. But for your case, I think you are right that a hook might be handy here. But I couldn't find an existing one that would be ideal, so I went ahead and added one.

I've added a new hook ProcessPageSearch::findReady() that should let you modify the selector before the search is performed. Then you can replace a placeholder like "date=foobar" with the actual part that you want. Here's how you might hook it from /site/ready.php or maybe better yet /site/templates/admin.php:

$wire->addHookAfter('ProcessPageSearch::findReady', function(HookEvent $event) {
  $find = "date=foobar"; // placeholder selector you want to find and replace
  $replace = "(date=''), (date>=today)"; // selector string you want to replace it with
  if(strpos($event->return, $find) !== false) {
    $event->return = preg_replace('/\b' . $find . '\b/', $replace, $event->return); 
  }
}); 

[fijha]

Thank you very much for making this hook. But for some reason the custom selector value I want to replace is not passing to the hook event. If I test the $event->return I always get status<2048, title%=test, limit=50.

Edit:

Actually, I thought I could use some random word as field name for replacement, but found out it needs to be a name of existing field, otherwise it won't pass (sanitizer?). So now it works!

Also date>=today doesn't work. I need to use variable date>={$today}.

[ryancramerdesign]

That's true that it's got to be a field name that PW recognizes, otherwise it ignores it. But you should be able to use date>=today. I did just test that here (from the API) and it worked, though that's a little bit different context. What is in your $today variable, an integer?

[fijha]

Yes, it works. Sorry, my bad… The reason why today wasn't working for me and $today = time() was is that I had to use now instead, because today does not return actual time.

[ryancramerdesign]

The keywords "today" and "now" are both recognized by strtotime(), and are both acceptable values for selectors referring to FieldtypeDatetime fields. But note that "today" and "now" are not the same thing. The keyword "today" refers to the beginning of today (midnight), whereas "now" literally refers to this moment, now, what is. So day/month/year part will be the same, but the hour/minute/second part will be different, except at midnight. :)

[fijha]

@ryancramerdesign When trying to save a page I'm getting an error:

Session: Error saving field "slideshow" - Page 16571 is not valid for slideshow (Page 16571 does not match findPagesSelector: title=selectorHookReplace, id=16571)

Expected behavior

Pages on save should be matched against hooked (replaced) selector, not the replacement selector inside selector string.

[netcarver]

@fijha Hello, can you let us know if this is still an issue with more recent versions of PW?

[fijha]

Yes, I tested in the most recent version (3.0.126)… the issue still exist. Ryan's solution works, but the page cannot be saved (as I mentioned in my previous comment), and because of that it's unfortunately unusable.

[ryancramerdesign]

@fijha It looks like you have the string selectorHookReplace in your selector string for the Page reference field? That needs to come out of there, as I think that's what's preventing the validation from matching. Perform your string replacement on something that will still enable the selector to match when it comes to validation. For instance, you might use title!='' or id>0 or something like that—anything that still allows the rest of the selector to match. Replace that id>0 (or whatever you chose) in your findReady() hook with whatever you are currently replacing title=selectorHookReplace with. That way, when the form is processed, the id>0 will still allow the rest of the selector to match (and your page selection to validate). This is necessary because you are hooking the finding of results in ProcessPageSearch, but you are not hooking the validation of results in FieldtypePage.

[fijha]

Thanks Ryan. This could be a solution, but wondering if it can cause some other problems. How can I make sure it affects only one field I want and not interfere with other fields? Because to my understanding the hook affects all ProcessPageSearch::findReady operations, for example admin PW search. This is the reason I was trying to use title=selectorHookReplace or something specific to avoid that.

[ryancramerdesign]

@fijha The requirement is that it just has to be a valid selector that will match. This is pretty easy to do with the id property because your front-end non-system page IDs start at 1000. So you could put it any id>n where n is less than 1000 (even negative numbers should work), and it should enable you to still perform your replacement while allowing the selector to match even when your replacement isn't present. So your code could (for instance) could replace id>123 or whatever number you choose to identify that particular scenario.

[fijha]

Yes, I understand it has to be a valid selector, but my point is what if I have 2 or more (same type) fields that needs to use id>123 – they all will be affected.

Or just for the sake of example, if I will use title%=home as a replacement and than using PW search with phrase "home", which is translated to title%=home, start=0, limit=15, include=all selector, the PW search would return different results, because title%=home will be replaced inside my hook.

So It could lead to "bugs". Or would it be possible to check within a hook the field name or template of the page I'm editing?

[ryancramerdesign]

You wouldn't want to use title%=home as your placeholder to replace because that would prevent it from matching pages when your hook isn't involved... it would be that same problem as your ttitle=selectorHookReplace all over again. You need to use a selector that will match anything like those I mentioned. Simply replace the id>123 from your hook the same way you were replacing title=selectorHookReplace. The difference is that id>123 can match anything without your hook being called, where as title=selectorHookReplace cannot matching anything. If you have two or more fields where you need to do this then just use different numbers according to your need. For instance, you might assume that selectorHookReplace is referred to with id>777, and some other is referred to with id>778, and so on. Replace the id=n according to the meaning that you define.

[fijha]

I totally get it. But, I mean, it's not super safe. There must be a better (less hacky) way.

Should I keep this issue open or maybe put it in the feature requests?

Would it require o lot of time and work to just make it possible to use more complex selectors with Page Reference Field, e.g. match blank field='' and use of OR selectors (date=''), (date>=now)?

[Toutouwai]

This is the reason I was trying to use title=selectorHookReplace or something specific to avoid that.

You should be able to use this sort of approach (i.e. insert some recognizable string that is unlikely to occur in a real search and check for it in your hook) - you just need to use a != operator so that this condition doesn't actually exclude any pages.

[fijha]

@Toutouwai I'm not sure why I didn't think about that. Thank you very much!

[ryancramerdesign]

I totally get it. But, I mean, it's not super safe. There must be a better (less hacky) way.

@fijha Actually it is super safe. The core even uses a similar technique when it needs to produce a non-empty selector that doesn't change the results. Since you are trying to hook into and change the way a page field selector validates results, it's going to be a hack either way, so I'd suggest this solution is a good way to do that. What @Toutouwai is also fine, though if your selector will ever be hitting the database unmodified I'd suggest sticking with using the id, as it won't add overhead whereas the field!=string solution could.

[netcarver]

@fijha Does Ryan's reply above allow you to close this issue now?

[reminders]

@netcarver set a reminder for Nov 10th 2019

[reminders]

👋 @netcarver, review this issue

[netcarver]

@fijha I'm closing this issue now. If you need it reopened, just post again here.