Normalize fragment encoding to avoid duplicate route fires (#4132)

[BigBalli]

Fixes #4132. Also addresses #4085 and #3941, which are the same
encoding mismatch through different code paths.

Problem

In Firefox (and historically Safari), location.href returns the hash
with percent-encoded non-ASCII characters even when navigate() was
called with the decoded form. After navigating to #search/大阪, the
next hashchange/popstate:

1. checkUrl() calls getFragment() → getHash() → reads
   search/%E5%A4%A7%E9%98%AA from location.href.
2. Compares against the cached this.fragment, which navigate() set
   to the decoded form search/大阪.
3. They differ → loadUrl() runs → the route fires a second time.

This breaks any i18n app whose URLs contain CJK, Cyrillic, accented
Latin, etc.: every navigation triggers analytics/fetches/state changes
twice.

Fix

Normalize on the decoded form everywhere a fragment is cached or
compared:

• History#start stores the initial fragment as
  decodeFragment(getFragment()).
• History#checkUrl compares decodeFragment(getFragment()) (and the
  iframe hash) against this.fragment.
• History#loadUrl writes decodeFragment(getFragment(fragment)) to
  this.fragment before dispatching.

decodeFragment is already designed to preserve literal %25, so it
is idempotent on values that are already decoded — safe to call on
the pushState path that already decodes via getPath().

Test

Added a regression test that constructs a History instance with a
mocked location.href containing the encoded form of search/大阪,
sets this.fragment to the decoded form (as navigate would), and
asserts that checkUrl() does not re-invoke the matching route
handler.

npm run lint passes.

[jgonggrijp]

This seems correct to me. 👍

@paulfalgout could you take a look as well?

[Copilot]

Pull request overview

This PR fixes a cross-browser fragment encoding mismatch where Firefox/Safari may expose percent-encoded non-ASCII hash fragments via location.href, causing History#checkUrl to detect a “changed” fragment and fire routes twice. It normalizes fragment caching/comparisons to the decoded form and adds a regression test for an encoded CJK hash fragment.

Changes:

• Decode and store the initial fragment in History#start for consistent caching.
• Compare decoded fragments in History#checkUrl (including iframe hash comparisons).
• Store the decoded fragment in History#loadUrl before dispatching handlers.
• Add a QUnit regression test for checkUrl() not double-firing on encoded non-ASCII hashes.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
backbone.js	Normalize fragment storage/comparison to decoded form across start/checkUrl/loadUrl to prevent duplicate route fires.
test/router.js	Add regression test ensuring checkUrl() does not re-fire a route when location.href contains an encoded non-ASCII hash.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

decodeFragment uses decodeURI, which throws URIError on malformed percent-escapes (e.g. a user manually enters a hash containing a stray % or incomplete UTF-8 sequence). By decoding unconditionally in start/checkUrl/loadUrl now, hashChange navigation that previously tolerated such URLs can start crashing the router. Consider wrapping the decode in a try/catch (returning the original fragment on failure) or adding a safe decode helper used by these call sites so invalid fragments don’t break history handling.

[paulfalgout]

so it's suggesting:

decodeFragment: function(fragment) {
  var raw = fragment;
  fragment = fragment.replace(/%25/g, '%2525');
  try {
    return decodeURI(fragment);
  } catch (e) {
    return raw;
  }
}

I suppose it could be breaking.. although seems unlikely

[paulfalgout]

so it's suggesting:

decodeFragment: function(fragment) {
  var raw = fragment;
  fragment = fragment.replace(/%25/g, '%2525');
  try {
    return decodeURI(fragment);
  } catch (e) {
    return raw;
  }
}

I suppose it could be breaking.. although seems unlikely

[paulfalgout]

fragment = this.getFragment(fragment);
this.fragment = this.decodeFragment(fragment);

Decode only the cached fragment for comparison; don’t decode the fragment you match routes against, or you change routing semantics.