cloudsearch.js doesn't work in Google Chrome

[DeeNewcum]

When you try to use the script on Google Chrome, it doesn't work at all.

If you check in the console errors, it says:

Uncaught ReferenceError: QueryString is not defined

[DeeNewcum]

It turns out that Google Chrome's implementation of the Greasemonkey engine doesn't implement @require, so the line at the top doesn't work at all:

// @require ...  query-string-parser.js

I see three possible ways to solve this, but none are particularly nice:

1. Add this code to load the second JS script. Downside: It's a bit of code (unless included minified) you have to paste in. Also you have to wrap the existing script in a call to this -- you have to pass a function-reference in as "onLoad", to be called after the second script is loaded. Not very clean.
2. Copy-n-paste the entire second JS script into the first script, verbatim. Downside: This is a little messy.
3. Wait until we get it converted to a RES Module. Since RES is a full-fledged module, it can have many separate JS files. Downside: It may be a while before this is complete.

You can see what the changes would look like for option 1 and 2 in my copy of the repo.

[DeeNewcum]

I use Google Chrome most of the time, and I would really like to be able to dogfood this without having to wait for it to work as a RES module. But the other options aren't so nice either.

Any thoughts?

[Sophira]

In my original version of the script (before I added it to the GitHub repository), I had included a minified version of the code in the script itself. I realised that it would be a lot cleaner to split it out, especially if I had other scripts that would rely on the same functionality.

Organisation-wise, option 1 is cleaner. However, there are some very important downsides which mean that I don't want to use that solution:

1. Unlike @require, where implementations of it cache the requested document, there's no explicit cache here - just the browser cache. This could lead to more requests than necessary, though obviously this depends on quite a few external variables such as the headers the server returns and how often the cache is cleared, etc.
2. More importantly, however, it injects both the referenced script and the executed function into the DOM as <script> elements. This is bad because it completely bypasses the sandbox environment that Greasemonkey (and I expect other implementations) implements. Generally speaking, it could also potentially cause conflicts with whatever scripts are used in the target page. In this case the conflicts are unlikely to be an issue, but I would not want to bypass the sandbox.

So, unless there's some way of making it safer, I don't think I'd want to use option 1. Option 2 is much safer, at the cost of a little organisational overhead and a less elegant manner of doing things. (And I'm suddenly very glad I rewrote the script to not use jQuery...)

I think the best solution is to minify the query string parser, strip out all the comments, and acknowledge the source ( http://unixpapa.com/js/querystring.html ) just above the paste. If you don't have any objections, I'll do that as soon as I can.

Of course, we still want to make the RES module in any case as I think it's a good idea. (And RES likely has its own query-string parser anyway which we should use, so porting that is unlikely to be an issue.)

What do you think?

[edit: Used a different URL for the sandbox environment link.]

[DeeNewcum]

Yeah, the sandbox-security-breaking issue is bad. It's great you noticed that.

I think the best solution is to minify the query string parser ...

Agreed.

RES does include a URL-parser, but the one you found is much nicer. So I assume when we integrate with RES, we'll continue using yours.

Now I'm eager to do the RES work, so we can tidy this code up.

[Sophira]

There's little point in duplicating work done elsewhere, and I suspect that it's less likely that an RES module gets approved if it does so. However, I'm going to suggest that RES uses this parser instead; this shouldn't be an issue as the author has explicitly disclaimed copyright, so it would be in the public domain. I'm not sure of the exact process that the RES team prefers, so I'll post to /r/Enhancement about it.

[Sophira]

Made a thread on /r/Enhancement: https://www.reddit.com/r/Enhancement/comments/2x4vjr/feature_request_use_jan_wolters_query_string/ .

[DeeNewcum]

I'm working on the RES module at: https://github.com/DeeNewcum/Reddit-Enhancement-Suite/commits/cloudsearch

Here's some additional notes.

[DeeNewcum]

I have Cloudsearch + RES working in Chrome and Firefox. I haven't tested it a ton yet, but it does seem to be working.

https://github.com/DeeNewcum/Reddit-Enhancement-Suite/commits/cloudsearch

(after you load a browser up, you can confirm it's working by going to: RES Settings Console > Posts > Cloudsearch Syntax, and just confirming that menu item appears)

[Sophira]

I'll pull the branch into my fork tomorrow and give it a test on Firefox!

[Sophira]

Okay, I'm taking a look at it now. A couple of things I'm noticing so far:

1. The addon version doesn't change the "I couldn't understand your query..." error - is this deliberate?
2. The module configuration seems to add a new "There are no configurable options for this module." section every time I go into it, and that spreads to the others too. In other words: Normally I can switch between one non-configurable module and any other module without seeing more than one, but with the Cloudsearch one it adds another message each time. After that's happened, I then see the same number of such messages on other non-configurable modules. Does that make sense? (see next comment)

Otherwise, I can't notice any problems thus far. I'll report back if I find any more!

[Sophira]

Regarding 2: Never mind, it does happen on other non-configurable modules. I think it must be a bug with the current Git version of RES. (Or maybe I did something wrong when building it, I don't know.) Sorry! I didn't notice it at first because it actually spreads to all modules, not just non-configurable ones.

[DeeNewcum]

1. Nope, not intentional. That's a bug.
2. Also, the tabindex doesn't seem to work. (confirmed in both Chromium and Firefox)