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

Tag cloud links do not escape the & character #48

Closed
nodiscc opened this issue Nov 8, 2014 · 12 comments · Fixed by #54
Closed

Tag cloud links do not escape the & character #48

nodiscc opened this issue Nov 8, 2014 · 12 comments · Fixed by #54
Assignees
Labels
bug it's broken!

Comments

@nodiscc
Copy link
Member

nodiscc commented Nov 8, 2014

sebsauvage#85

When a tag containing "&" is given to a link (such as "tips&tricks"), the search of this tag returns no element.

@nodiscc nodiscc added the bug it's broken! label Nov 8, 2014
@nodiscc
Copy link
Member Author

nodiscc commented Nov 8, 2014

I think we should prevent storing & in tags (maybe replace it automatically with another character) instead of trying to fix the search function.

@e2jk
Copy link

e2jk commented Nov 8, 2014

I agree with not storing tags containg &.
Either stip it, or replace & by "AND" (or its localized equivalent, the day that we have internationalized Shaarli)

@nodiscc
Copy link
Member Author

nodiscc commented Nov 8, 2014

@e2jk would an underscore _ be ok? No need to localize this then.

@e2jk
Copy link

e2jk commented Nov 8, 2014

I have no strong feelings. I would never put an & or any kind of weird characters in my tags anyway...

@virtualtam
Copy link
Member

There are a handful cases where & can have a meaning, that would be lost if replaced:

  • b&w
  • r&b

For most usages, splitting compound tags into single tags should be fine, e.g. "tips&tricks" becomes "tips tricks".

Using only single tags is also more convenient when performing searches...

Did I tag this comic "starwars"? Er.. nope, let's try "star-wars"... still nothin', was it "star_wars"? Crap...

@nodiscc
Copy link
Member Author

nodiscc commented Nov 9, 2014

cases where & can have a meaning, that would be lost if replaced

True. r_b would lose the meaning.

splitting compound tags into single tags should be fine

No. If the user enters a single tag it should be kept single. In your case we would end up with an r and a b tag. Or tips and tricks tags. This is worse than replacing.

I think the best way is to just strip the & char for now, and document this in the user manual. Or the search function should be fixed. If someone can propose a simple fix for this bug we could merge it.

@nodiscc
Copy link
Member Author

nodiscc commented Nov 9, 2014

Did I tag this comic "starwars"? Er.. nope, let's try "star-wars"... still nothin',

Stripping or replacing will cause problems like this, yes. I think it's actually better than the current situation (can't search/filter tags with & at all). This will be less annoying if we can re-add tag autocompletion in the tag search field (#49)

@virtualtam
Copy link
Member

Btw, is this issue confirmed? I have no trouble adding and searching tags containing &, whether it's at the beginning, within, or at the end of the tag name...

@nodiscc
Copy link
Member Author

nodiscc commented Nov 9, 2014

Uh. You're right I can't reproduce it anymore...

@nodiscc
Copy link
Member Author

nodiscc commented Nov 9, 2014

Ok I was able to reproduce this. Good news this is not as bad as originally reported.

Using either the filter by tag field or clicking the tag in the link list works perfecly (the tag r&b is properly escaped to ?searchtags=r%26b

But when using the tag cloud the r&b tag points to ?searchtags=r&b, which fails. So this is just an escaping problem in the tag cloud. If you think of other ways to cause this bug, please report them.

@nodiscc nodiscc changed the title Tags with the & character are not searchable Tag cloud links do not escape the & character Nov 9, 2014
@virtualtam
Copy link
Member

Looking at the whole code, the only occurence of &searchtags= that is not chained with urlencode() is in tpl/tagcloud.html

The Picture Wall could also be concerned, as (though this is not used) it is possible to append &searchtags=[...] to the page URL to filter displayed pictures by tag

@nodiscc
Copy link
Member Author

nodiscc commented Nov 9, 2014

Thanks for checking @virtualtam

I've proposed a fix at #54 (I don't know why htmlspecialchars was used in the first place)

The Picture Wall could also be concerned, as (though this is not used) it is possible to append &searchtags=[...] to the page URL to filter displayed pictures by tag

We can not prevent users to input & in the wrong place in the URL bar of their browser. The usual way to do picture wall filtering is to click on Picture wall after doing a search in the main link list.

@e2jk e2jk closed this as completed in #54 Nov 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug it's broken!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants