history navigation fix following addition of decodeURI in rawth

[geonanorch]

This is a follow-up on issue #155: now that router paths are decoded, the comparison logic in dom.js is broken for URLs containing spaces:

const onRouterPush = path => {
  const url = path.includes(defaults.base) ? path : defaults.base + path
  const loc = getLocation({})
  const hist = getHistory()
  const doc = getDocument()

  // update the browser history only if it's necessary
  if (hist && url !== loc.href) {                   ////////////////// this comparison is broken
    hist.pushState(null, doc.title, url)
  }
}

In the example above url is decoded, but loc.href is not. This causes the following browser history navigation failure in my case:

• start with /
• go to /my space using <a href="/my space">link</a>
• click the browser 'back' key ==> back to /
• click the browser 'forward' key ==> on my space again
• click the browser 'back' key (any number of times) ==> nothing happens

I eventually realized that the first 'forward' resulted in a push to /my%20space, which prompted to code above to add one more page /my space to history. Clicking'back' then restores /my%20space, causing again the /my space entry to be added.

This PR wraps loc.href above with decodeURI(). I tried to review all usages of location.href, which prompted me to also factorize further to getLocation() all accesses to window.location. My understanding is that getLocation() is not exported from the router, hence the change of (default) behavior is ok.

Side-question: should rawth not be using decodeURI instead of decodeURIComponent ? (as it is applied to multiple components)

[coveralls]

Coverage remained the same at 100.0% when pulling 680ed40 on geonanorch:main into 98283eb on riot:main.

[geonanorch]

One more note about this: I tested that router.push() works both with encoded /my%20space and decoded /my space links, and did not try to change other areas in the code where encoded urls are present. But to be honest I could not figure out why both encoded and decoded work...

[GianlucaGuarini]

Thank you for your PR it looks good but we need to fix a few things before merging it.

Side-question: should rawth not be using decodeURI instead of decodeURIComponent ? (as it is applied to multiple components)

@geonanorch you are right would you mind creating a PR on rawth first?

I will come back to this afterwards, Could you please write also a test to check the issue? This would help me enormously

[geonanorch]

Sorry for the late reply, not a lot of time right now. I would be happy to give back to Riot.js, will aim to raise that PR for rawth this week.

[geonanorch]

Sorry @GianlucaGuarini, finally managed to make progress on this, see PR-5 in rawth.
Let me know how you want to proceed with this riot PR.

[GianlucaGuarini]

Thank you, the issue has been solved thank to your patch in v7.1.1