Gapminder / gapminder-tools-vizabi

❌ [OBSOLETE] Don't go here. Use ng2-tools-page
GNU General Public License v3.0
3 stars 0 forks source link

Angular encodes url-state #172

Open jheeffer opened 8 years ago

jheeffer commented 8 years ago

Angular on Tools Page has a onhashchange event which encodes characters in the hash of the url. It doesn't break anything but the encoding is unnecessary, costs resources and causes less readable permalinks.

https://github.com/Gapminder/vizabi/issues/1339#issuecomment-201621712 (point 2)

reproduce:

  1. change indicator
  2. look at url with %2F as encoded /

How to get to the angular event

  1. enter in console of (chrome) dev toolbar: window.onhashchange = function() { debugger; }
  2. change indicator
  3. first event is TP reacting to Vizabi event
  4. second event is Angular reacting to TP setting url hash
  5. Look in stack trace for second event
jheeffer commented 8 years ago

We just found out that it probably breaks the back button because each onhashchange gets recorded in browserhistory.

jheeffer commented 8 years ago

Possible solution. This is from the angular library on toolspage:

/**
 * We need our custom method because encodeURIComponent is too aggressive and doesn't follow
 * http://www.ietf.org/rfc/rfc3986.txt with regards to the character set (pchar) allowed in path
 * segments:
 *    segment       = *pchar
 *    pchar         = unreserved / pct-encoded / sub-delims / ":" / "@"
 *    pct-encoded   = "%" HEXDIG HEXDIG
 *    unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"
 *    sub-delims    = "!" / "$" / "&" / "'" / "(" / ")"
 *                     / "*" / "+" / "," / ";" / "="
 */
function encodeUriSegment(val) {
  return encodeUriQuery(val, true).
             replace(/%26/gi, '&').
             replace(/%3D/gi, '=').
             replace(/%2B/gi, '+');
}

/**
 * This method is intended for encoding *key* or *value* parts of query component. We need a custom
 * method because encodeURIComponent is too aggressive and encodes stuff that doesn't have to be
 * encoded per http://tools.ietf.org/html/rfc3986:
 *    query       = *( pchar / "/" / "?" )
 *    pchar         = unreserved / pct-encoded / sub-delims / ":" / "@"
 *    unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"
 *    pct-encoded   = "%" HEXDIG HEXDIG
 *    sub-delims    = "!" / "$" / "&" / "'" / "(" / ")"
 *                     / "*" / "+" / "," / ";" / "="
 */
function encodeUriQuery(val, pctEncodeSpaces) {
  return encodeURIComponent(val).
             replace(/%40/gi, '@').
             replace(/%3A/gi, ':').
             replace(/%24/g, '$').
             replace(/%2C/gi, ',').
             replace(/%3B/gi, ';').
             replace(/%20/g, (pctEncodeSpaces ? '%20' : '+'));
}

so, they use encodeUriSegment to encode the fragment, which uses encodeUriQuery. They correctly say that encodeURIComponent (native function) is too aggressive

but they don't decode everything on which encodeURIComponent is too aggressive, including /

encodeURIComponent('/')
>"%2F"

I think you could just add that case to their encodeUriQuery function

replace(/%2F/gi, '/').
jheeffer commented 8 years ago

Or maybe there's an Angular function to change the url which will cause only 1 step in history. You can use that instead of the current native code, which (together with Angular) causes the 2 steps in history. (Assumption is that 2 steps in history breaks the back button)

            function onPersistentChange(evt, minModel) {
              var minModelDiff = Vizabi.utils.diffObject(minModel, initialModel);
              window.location.hash = urlon.stringify(minModelDiff);
            }

https://github.com/Gapminder/gapminder-tools-vizabi/blob/development/client/src/js/services.js#L39

maybe $location.hash(urlon.stringify(minModelDiff)); https://docs.angularjs.org/api/ng/service/$location

jheeffer commented 8 years ago

Here you can see they did it the right way in Angular 2 codebase

https://github.com/angular/angular/blob/2.0.x/modules/@angular/http/src/url_search_params.ts#L33-L44