Page MenuHomePhabricator

Use better encoding for MediaViewer URLs
Closed, ResolvedPublic

Description

Migrated from: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/371

Narrative

As a user, I want share URLs which are easy to handle and do not break in various environments

Acceptance Criteria

  • URL should work even if the file name has exotic characters and the browser has weird ideas about how to encode/decode URLs (esp. old IE)
  • URL should be autolinked in clients which do that sort of thing (e.g. Gmail) without the end being chopped off
  • URL should work even if the file name contains special characters (especially a / sign)
  • URL should be possible to generate from the file name in a wiki environment (via {{urlencode}} or {{anchorencode}} for example), preferably without having to resort to Lua
  • if it is possible to do all that and still have a URL which is readable in modern browsers (non-Latin languages do not become a hex soup), that would be great. It is probably not, though.

Related Bugs

Related Stories

Related Changesets

Event Timeline

MingleTerminator raised the priority of this task from to High.Dec 8 2014, 4:57 PM
In mingle on 2014-03-19 at 19:21:00, @Tgr wrote:

Splitting from #206

In mingle on 2014-04-11 at 23:46:10, @Tgr wrote:

Fun fact: {{anchorencode}} is non-reversible as it encodes . to . and non-ASCII chars to .NN (where NN is the hexa code).

In mingle on 2014-04-11 at 23:49:36, @Tgr wrote:

If we go for anchors which are valid HTML4 ids, that leaves us with [A-Za-z0-9:._-]. (Even the / mark is invalid, could be replaced with : which still looks OK.)

In mingle on 2014-04-12 at 00:30:29, @Tgr wrote:

On reflection ids are not relevant here at all. The WHATWG URL standard http://url.spec.whatwg.org/#writing allows [A-Za-z0-9!$&'()*+,./:;=?@_~-] in the fragment identifier and the URI RFC http://tools.ietf.org/html/rfc3986#section-3.5 allows [A-Za-z0-9/?:@_.~ "!" / $&'()*+,;=-]; both standards suggest percent-encoding.

In mingle on 2014-04-12 at 00:47:48, @Tgr wrote:

{{urlencode}} uses wfUrlencode() which percent-encodes everything except ;:@$!*(),/

encodeURIComponent leaves in place -_.!~*'()

encodeURI leaves in place ;,/?:@&=+$# plus all those left by encodeURIComponent.

mw.util.rawurlencode leaves in place !'()*~ plus all those left by encodeURIComponent.

mw.util.wikiUrlencode leaves in place _:/ plus all those left by rawurlencode.

In mingle on 2014-04-12 at 00:51:12, @Tgr wrote:

oops, misread that. mw.util.rawurlencode encodes absolutely everything non-ASCII. mw.util.wikiUrlencode leaves in place _:/ and nothing else non-ASCII.

In mingle on 2014-04-12 at 00:57:28, @Tgr wrote:

Awright, one more try.

mw.util.rawurlencode leaves in place -_.
mw.util.wikiUrlencode leaves in place -_.:/ and converts spaces to underlines.

In mingle on 2014-04-12 at 01:03:32, @Tgr wrote:

If we percent-encode/decode (which seems to be the obvious thing to do - the other alternative would have been {{anchorencode}}'s dot-encoding but that's broken by design) then the exact details do not matter much since decodeURIComponent will decode every %XX pattern, so it is up to us what characters to whitelist (as long as % is exceluded), and future changes will not break backwards compatibility as long as they percent-encode at least percent signs. To avoid adding yet another encoding, mw.util.wikiUrlencode seems to be the way to go.

In mingle on 2014-04-12 at 01:07:32, @Tgr wrote:

That means MediaViewer URLs could be generated on wiki side with something like [{{fullurl:PAGENAME}}MediaViewer/{{urlencode:IMAGENAME}}]. Not beautiful but works.

In mingle on 2014-04-15 at 01:33:47, @Tgr wrote:

Firefox displays percent-encoded hashes nicely; other browsers do not. https://www.w3.org/Bugs/Public/show_bug.cgi?id=10410 seems to say (although not very clearly) that Firefox's behavior is the correct one, so this will hopefully work everywhere in the future.

In mingle on 2014-04-15 at 02:17:05, @Tgr wrote:

https://gerrit.wikimedia.org/r/#/c/125891/
https://gerrit.wikimedia.org/r/#/c/125932/

One question left open is whether to canonize URLs (that is, do a replaceState if the current URL can be percent-decoded back to a valid filename, but it is not the same as the one the viewer would generate, e.g. because it was created by {{urlencode}} whose choice of which characters to leave unencoded is slightly different).

In mingle on 2014-04-16 at 02:52:01, @Tgr wrote:

This would also fix the bug we currently have that the share/embed links have the wrong format (no File: prefix). If we cannot merge this by Thursday, we probably want to backport a quick fix. (Maybe even if we can merge, since that's still a week delay.)

In mingle on 2014-04-17 at 22:10:10, @MarkTraceur wrote:

Tested, verified on beta.