search

nornagon / cdda-guide

The Hitchhiker's Guide to the Cataclysm
https://cdda-guide.nornagon.net
GNU General Public License v3.0
112 stars 27 forks source link

[Feature request] OpenSearch Manifest #169

Closed Djent- closed 6 months ago

Djent- commented 6 months ago

OpenSearch is a way for a website to advertise to users' browsers that a field provides a search engine functionality. This would be useful so users (like me) could add Hitchhiker's Guide as a built-in search engine to their browsers, following the instructions https://support.mozilla.org/en-US/kb/add-or-remove-search-engine-firefox

Implementation would require modifying the template OpenSearch XML manifest file and adding a <link ... to it in the webpage <head>, as explained in the documentation below: https://developer.mozilla.org/en-US/docs/Web/OpenSearch

nornagon commented 6 months ago

Yeah, this is a good idea, I'll add it!

Djent- commented 6 months ago

Thanks nornagon for the fast response! I saw you added a few commits addressing OpenSearch last night. For me (tried using Firefox and Chromium) it still doesn't work correctly, for some combination of the following reasons:

nornagon commented 6 months ago

Regarding the caching, I don't think the browser will go through the service worker to fetch /opensearch.xml. So I don't believe that's an issue, though if you have evidence to the contrary I'm interested to see it!

As for the other two issues, hopefully 8c8913b24710fbfddef8730bcb4cb2fb5cdb4267 clears them up.

Djent- commented 6 months ago

Adding the search engine to browsers works correctly now.

However, searching using the search engine doesn't work.

nornagon commented 6 months ago

Ah, thanks for pointing those out! 🤦🏻

The template should be fixed with 1c36660, but I'm not exactly sure what to do with the + issue. Chrome seems to encode spaces as %20, which works correctly. If you type e.g. +P into the search box, Chrome resolves that to https://cdda-guide.nornagon.net/search/+P, which correctly searches for overpressure ammo. If I decode + as space, then this won't work correctly in Chrome :/ This kinda seems like a Firefox bug. The spec says the query should be url encoded, and encodeURIComponent('search term') gives "search%20term".

... that said, https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/decodeURIComponent#decoding_query_parameters_from_a_url suggests replacing + by space when decoding from a URL, so uh. idk.

nornagon commented 6 months ago

2126188 is an attempt to handle the conflict gracefully, let's see how that goes.

Djent- commented 6 months ago

Looking forward to testing the search again once it goes live.

We are definitely in the bowels of Firefox's implementation details. It appears the question of "plus encoding", which is used for application/x-www-form-urlencoded vs "percent encoding", which is what the URL RFC describes has been known for a while by Mozilla https://support.mozilla.org/mk/questions/1396405

nornagon commented 6 months ago

It should already be live. I had to remove and re-add the search engine in Chrome for it to update, but it seems to be working. Unfortunately I was correct that searching for "+P" through the search plugin ends up searching for " P" in Chrome, not sure how it's going in Firefox. I guess I could test the UA, but that feels like such a hack!

Djent- commented 6 months ago

It all seems to work in Firefox! HHG understands that + is a space, Firefox percent encodes + to %2B which searches for over-pressured ammunition correctly. It is a bummer that this caused a regression for Chromium-based browsers since that's what most of the internet is using.

Checking the User-Agent probably is the way to go for this situation. Short of using a purpose-built User-Agent categorization library, looking for like Gecko is a good way to separate out the non-Firefox from the Firefox. I too wish we could move on from the days of checking User-Agent strings though

The last major desktop browser to test would be Safari, which I don't have a good way to help with. We can presume Safari's behavior falls into either Chrome's or Firefox's.