938109 - SOMETIMES %26 is decoded when tags have an "&" sign results in ineffectual search functionality

Status: RESOLVED FIXED

(Webmaker Graveyard :: webmaker.org, defect)

Description

[Laura Hilliger [:epilepticrabbit]]

I've been tagging /teach makes with WebLitStd competencies. I documented the procedure here: https://support.mozilla.org/en-US/kb/how-tag-webmaker-makes-web-literacy-standard

for "Design & Accessibility" the tag is "design&accessibility", but when you use the "&" in the tag, it is decoded in %26. Clicking on the "design%26accessibility" tag results in an appropriate URL search (i.e. https://webmaker.org/en-US/search?type=tags&q=design&accessibility ) but the search bar shows only the first part of the tag (i.e. "design") (Screenshot, compare URL w/ our search bar: 
http://i.imgur.com/cwduoRc.png )

Also, the %26 is NOT decoded for "sharing&collaborating", that one decodes as "&" (i.e. identical to the tag), but the Search issue is the same.

The easiest way to see all this weirdness is to look at the MozTeach search: https://webmaker.org/en-US/search?type=user&q=mozteach and pay attention to makes tagged with "design%26accessibility", "coding%26scripting" or "sharing&collaborating"

Comment 1

[Chris DeCairos (:cade)]

The issue here may be where the ampersand is being entered. i.e. in Thimble vs in the Make Editor

I'll take a look.

Comment 2

[Chris DeCairos (:cade)]

Yeah, confirmed, adding a tag with an ampersand to the make using thimble's tag tools will cause it to be escaped before being saved to the MakeAPI.

Not sure how we want to handle this, because we do want to escape user input from the app.

Short term, add tags containing ampersands using the Admin tool.

Comment 3

[Mark Goodwin [:mgoodwin]]

The correct thing to do here is to ensure the data is correctly encoded for output wherever it's used. See also https://bugzilla.mozilla.org/show_bug.cgi?id=938209#c9

Comment 4

[Chris DeCairos (:cade)]

Attachment: https://github.com/mozilla/thimble.webmaker.org/pull/287

Comment 5

[Chris DeCairos (:cade)]

Attachment: https://github.com/mozilla/popcorn.webmaker.org/pull/320

Comment 6

[Pomax]

Comment on attachment 832274 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/287

renders decoded HTML as text rather than as html. R+

Comment 7

[Pomax]

Comment on attachment 832328 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/320

renders decoded HTML as text rather than as html. R+

Comment 8

[[github robot]]

Commit pushed to master at https://github.com/mozilla/thimble.webmaker.org

https://github.com/mozilla/thimble.webmaker.org/commit/98152e4735ba7a16c3ae133b62c52f53b867db34
Bug 938109 - escape decoded tags

Comment 9

[Chris DeCairos (:cade)]

Production Popcorn Maker and Thimble now properly escape tags that are output in the UI.

Laura: For security purposes we've had to disallow '&' from being included in tags. I would suggest using '-', or separate tags rather than compound ones. Work is still to be done to fix the tagging problems in thimble and popcorn arising from the way we've implemented tutorials.