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

[fix] using hash symbol in search query causes broken display #110

Merged
merged 2 commits into from
Dec 21, 2020

Conversation

phproberto
Copy link
Contributor

When submitting a search containing the hash symbol the query request is truncated and you get JSON used as text result.

Sample query: the #1 problem

Expected AJAX query: s?q=the%20%231%20problem&l=20&sl=300&search_type=auto&ajax=true
Actual query: s?q=the

The issue comes because without encoding uri parameter the query becomes:

/s?q=the #1 problem&l=20&sl=300&search_type=auto&ajax=true

And everything after the hash symbol is removed.

This introduces: encodeURIComponent() so everything is encoded before making the request.

See:
https://developer.mozilla.org/es/docs/Web/JavaScript/Referencia/Objetos_globales/encodeURIComponent

@phproberto
Copy link
Contributor Author

BTW you can see that a lot of sites have that issue:

https://www.google.com/search?q=%22Powered+by+tntsearch%22

And search on any result page something like: this#1. You will see the JSON Markup shown.

@w00fz
Copy link
Member

w00fz commented Dec 21, 2020

This looks good to me! Good catch and thank you for the fix.

Did you recompile the js via npm run prod? I just wanted to make sure before I merge this in.

@phproberto
Copy link
Contributor Author

phproberto commented Dec 21, 2020

I did yarn dev & yarn watch but now I've executed yarn prod just to ensure is ok

@w00fz w00fz merged commit 2daa944 into trilbymedia:develop Dec 21, 2020
@w00fz
Copy link
Member

w00fz commented Dec 21, 2020

Perfect. Thank you 👍

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

Successfully merging this pull request may close these issues.

2 participants