[All] remove all usage of escape()/unescape()

[agentlame]

This will require serious compatibility testing.

[creesch]

@agentlame @TheEnigmaBlade @Dakta @JordanMilne @LowSociety

I could use some input here. We changed all unescapes() in 3.0.4 to decodeURIComponent. Reasoning being that it prepares people for when reasons starts to get encoded by it's counterpart. Which works fine for the most part except for a few subreddits where it results in

 Uncaught URIError: URI malformed

Caused characters like these quotes “” and probably some other (non utf-8?) characters as well.

Which is slightly problematic (Although a small group of subreddits) and if at all possible I want to account for that since it otherwise means manually fixing these wiki pages.

Anyone got ideas on how to approach this in a way that doesn't break a ton of shit? :)

[JordanMilne]

Haven't looked at the code, but does it know to use unescape() for old schemas and decodeURIComponent() for new stuff? Stuff that comes out of escape() won't necessarily work with decodeURIComponent().

decodeURIComponent(escape("“"))
URIError: malformed URI sequence

Any kind of encoding is probably unnecessary. IIRC the only reason escape() and co were a hack around the fact that reddit HTML encodes all quotes and brackets in JSON it serves up. Anywhere you would have done the encode() / decode() dance, don't encode it at all and just decode whatever you get back if you want to treat it as raw HTML ( var decoded = $.parseHTML(whatever).text() or something.)

Normally I'd say you should be doing the HTML decoding as it comes in, but a bunch of places within toolbox implicitly rely on the escaping being left in for safety.

So, you probably need something like:

function decodeSchemaField(schema, field_val) {
  var ver = schema.version;
  if(ver <= OLD_VER) {
    return unescape(field_val);
  } else if (ver <= NEWER_VER) {
    return decodeURIComponent(field_val);
  } else {
    // Or however you want to decode HTML entities
    return $.parseHTML(field_val).text();
  }
}

// No encoding step necessary when we just decode what reddit sends us.

[creesch]

Well, I didn't really go with a smart schema solution since I didn't think we would need a new schema for it. So basically I went for the bruteforce method and plainly replace unescape.

So

        var removalReasonText = unescape(config.removalReasons.reasons[i].text);

became

        var removalReasonText = decodeURIComponent(config.removalReasons.reasons[i].text);

We do need to escape things though otherwise if people use quotes it will break our own json.

[creesch]

Oh btw, places where it is used

https://github.com/creesch/reddit-moderator-toolbox/blob/master/modules/removalreasons.js#L226

https://github.com/creesch/reddit-moderator-toolbox/blob/master/modules/config.js#L117

https://github.com/creesch/reddit-moderator-toolbox/blob/master/modules/config.js#L384

[creesch]

moving this to 3.2, clearly this needs a bit more thought.

[creesch]

Sigh... If we ever want to do this I fear we need to do a new version schema...

[creesch]

Still relevant but does require a schema change if we ever tackle this with a two stage rollout. I tagged the modules that make use of versioned json schemas, I am not sure if they are all affected. It might just be removal reasons and/or macros.

[eritbh]

rel #199? (just going through oldest open issues)