search

codeforgermany / click_that_hood

A game where users must identify a city's neighborhoods as fast as possible
http://click-that-hood.com
MIT License
450 stars 639 forks source link

ignore non-city query params #344

Closed epaulson closed 2 years ago

epaulson commented 2 years ago

CTH has gone back and forth in how it handled the URL for linking right to a game for a specific neighborhood. Originally it was query strings (e.g. click-that-hood.com/?africa), and in #108 and #233 it got changed to resource names (e.g. click-that-hood.com/africa), and then recently Falco changed it back to query strings, because that makes hosting the game as a static webpage easier.

(In a clever approach for backwards compatibility to still support URLs with the "nicer" resource names now that the game is hosted via static html on github pages, the game index.html is also copied to the 404.html page, so when someone visits click-that-hood.com/africa, it's technically a 404 not found error, but as the game is also served as the 404 page, the game looks to see what you were trying to visit and plays that game)

I didn't go digging to see the full history here, but the game has also supported not just blank params, like ?africa, but also a key-value version, e.g. click-that-hood.com/?location=africa and ?city=africa - I don't know if it always did that or if it got added sometime after the initial version of the game)

But at any rate, since 2014 basically everything has just worked - cth.com/africa, cth.com/?africa, and cth.com/?location=africa

Several years ago Facebook added trackers to outbound links, e.g. every link shared on facebook became cth.com/africa?fbclid=xxxyyyzzz. It's annoying and I think technically isn't following the specs (I don't think the format of the query string is actually specified, and while it's usually true that it's formatted as k1=v1&k2=v2 and sites normally just ignore unknown keys, I don't actually think that's in the specs). At any rate, Facebook is doing it and so the rest of us who just live in Zuck's metaverse have to deal with it.

The change FB made worked fine for the resource-name version - https://click-that-hood.com/africa?fbclid=123234 is fine because the javascript code looks first for location.pathname which doesn't have the query params. However, it breaks the query-param version of the game, because for the case where we're not using the key/value option (e.g. we're not looking for city=africa or location=africa) - the game assumes the entire search query is the city name and doesn't try to parse it first. And since October of 2021 in https://github.com/codeforgermany/click_that_hood/commit/2cf5d7918b45b7218bec7914503e1c234468e4e4 the default for CTH is to use the query string version as the default URLs the game creates.

This PR fixes mangled URLs that have extra query parameters in the URL by adding a new fallback that checks the query string after the query string has been parsed and looks for a key that matches a cityname, so cth.com/?africa&fbclid=xxxx should work now. (It even works if you for some reasons someone adds a value, e.g. cth.com/?new-york-city-boroughs=bigapple though I don't know why that would ever happen )

In the unlikely event that someone uses two citynames in the URL, the last city is the usually that's used, e.g. /?new-york-city-boroughs&africa will usually play the Africa version of the game - but that depends on the forEach order and I don't think Javascript guarantees the order of forEach.

epaulson commented 2 years ago

@fnogatz any thoughts on this one? I saw your update on twitter a while back so if you're still not up to it, I completely understand.

epaulson commented 2 years ago

Hi @fnogatz - I'm really hoping to get this merged in soon. Is there anyone else who can review and potentially commit it?

fnogatz commented 2 years ago

Merged! Thank you for the detailed explanation and fix. I slightly rewrote the snippet to use a for-of-loop, which avoids additional variables and closures:

https://github.com/codeforgermany/click_that_hood/blob/0f16ce15b5f1ae77958a077669fb4201fd3cfc53/public/js/scripts.js#L1968-L1977

epaulson commented 2 years ago

Thank you! I didn't use the for-of and let because I wasn't sure how far back we were targeting for browser/javascript versions, so I went with something I knew would work with older browsers.

(something I want to do in the near future is learn how to use js build tools so we can have a transpilation step and use new javascript features as soon as we want)