Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

canDeactivate Hook Triggered Twice if the Destination Contains Unencoded Characters #630

Open
robpal94 opened this issue Mar 8, 2019 · 2 comments

Comments

@robpal94
Copy link

robpal94 commented Mar 8, 2019

I'm submitting a bug report

  • Library Version:
    1.6.3

Please tell us about your environment:

  • Operating System:
    macOS 10.14.2

  • Node Version:
    8.11.1

  • NPM Version:
    6.4.1

  • Browser:
    Electron/Chromium

  • Language:
    TypeScript 2.9.2

Current behavior:
The canDeactivate() hook is called twice whenever the destination route's fragment contains an unescaped character such as s space or letters like 'ä'.

If the hook is called the first time, the fragment of the passed NavigationInstruction is the unescaped string, which contains the raw special characters mentioned above.

After returning from canDeactivate(), the hook is called again.
This time, the fragment contains the encoded route.

Note: This basically always happens when using a space character in the destination url but only happens sometimes, when the unencoded character is another one, such as 'ä'. This makes the behavior of the canDeactivate() hook kinda random.

I know that there was already a fix for this in #616 ,but it seems that there are still problems when using unencoded special characters in the destination uri.

Expected/desired behavior:

  • Only call the canDeactivate() hook once, regardless of the route's fragment.
@bigopon
Copy link
Member

bigopon commented Apr 21, 2019

This can and should be fixed from history-browser module. The target is here https://github.com/aurelia/history-browser/blob/4cf18cf4bef2461915b94d0b4e9babd975b77cf3/src/browser-history.ts#L274

Basically change from

return '/' + fragment.replace(routeStripper, '');

to

return decodeURIComponent('/' + fragment.replace(routeStripper, ''));

@davismj
Copy link
Member

davismj commented May 7, 2019

I couldn't reproduce this issue. Is there a repro?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants