Page MenuHomePhabricator

mw.Uri does not roundtrip fragment characters
Closed, DeclinedPublicBUG REPORT

Description

(new mw.Uri('https://en.wikipedia.org/wiki/Main_Page#a/b/c')).toString()
// "https://en.wikipedia.org/wiki/Main_Page#a%2Fb%2Fc"

The slash is a valid character in a fragment and there is no reason to encode it; a fragment with a raw slash and a fragment with an encoded slash are different for most purposes, such as navigation or programmatic equality checks, so e.g. location.href = uri; and location.href = new mw.Uri( uri ).toString(); will result in different behavior if uri has a fragment used for anchoring or routing.

The same problem exists with various other characters, but with the slash it is especially bad since it's used extensively for routing in MobileFrontend. So, any kind of URL manipulation, even if completely unrelated to fragments (such as adding or removing a query parameter) will break MobileFrontend routes.

See also T103379: Add 'url' module for native URL constructor (with conditionally loaded polyfill) which proposes a new class with spec compliant behavior.

Event Timeline

I'm not sure what to do about it. Fixing the code to only encode characters that need to be encoded would be easy, but maybe some code already relies on the current broken logic? Hard to imagine how/why, but mw.Uri is an ubiquitous class so reviewing all the places where it's used would be a huge effort.

Indeed. This is likely a compat nightmare.

I suggest focussing instead on T103379: Add 'url' module for native URL constructor (with conditionally loaded polyfill).

new URL('https://en.wikipedia.org/wiki/Main_Page#a/b/c').toString()
"https://en.wikipedia.org/wiki/Main_Page#a/b/c"

Declining. I consider mw.Uri now frozen for compatibility to anything other than trivial and obvious bug fixes. It was ahead of its time and ahead of the WHATWG URL specification.

We can now tarnsition to the native URL API, starting with edge cases where these differences caused issues.