22

If you use "Insert hyperlink" (Ctrl+L or the link toolbar button), it modifies the URL you provide by replacing + with %20 if there's a query part, including in the fragment identifier. This breaks some URLs. Since + is valid in both the query part and the fragment identifier per the RFC, it should be left alone.


Details:

Here's an example of a URL it breaks:

Notice how the first one doesn't scroll down to the element, and the second one does. In the first one, I used Insert Link and pasted https://output.jsbin.com/qadofisoye/1?q=blah#foo+bar, which it converted to https://output.jsbin.com/qadofisoye/1?q=blah#foo%20bar. I manually corrected the second one.

This bug currently makes it hard to post links to TypeScript's new playground (as they get broken if they contain a +), but it's not limited to that as you can see above.

It appears to only do this if the URL contains a query part. https://output.jsbin.com/qadofisoye/1#foo+bar doesn't get broken, for instance. So this appears to relate to trying to do some kind of normalization on the query part. But note that scheme-based normalization does not apply to fragment identifiers, and a quick bit of research suggests that changing + to %20 in the query part is also both unnecessary and potentially problematic:

  1. In most query strings, + is synonymous with %20, so the change is pointless (and lengthens the URL unnecessarily) in that majority case.
  2. In a query string where + is not the same as %20 (because of the largely-deprecated indexed search), the change breaks the URL. (At least, according to Wikipedia, it's not something I've been deep into.)

So given it doesn't do anything useful, but does cause trouble sometimes, it should just leave the + alone.

5
  • 8
    It does require a query string. The code uses regex, which successfully identifies when a query string was entered, but doesn't attempt to identify when it was exited with a fragment. More interestingly, whoever wrote this code already knew this was unnecessary: "// Not strictly necessary, but identical behavior to the previous version of this function." Seems like we should just delete that short bit rather than complicating it more.
    – animuson StaffMod
    Commented Jun 27, 2019 at 14:10
  • 1
    @animuson: While they're at it, SE might consider fixing the non-BMP Unicode URL bug as well, since it's caused by the same regexp replacement. I have a simplified version in SOUP that doesn't suffer from either issue, and is probably faster too. Commented Jun 30, 2019 at 21:35
  • 2
    The real problem here is that %20 decodes to a space. If you want to encode a + it has to be %2b! Commented Jul 23, 2019 at 14:47
  • 3
    @animuson "we should just delete that short bit" - agreed, what can we do to make this happen? Commented Oct 2, 2022 at 13:22
  • @animuson - Any chance you could ping the people who could fix this? It comes up often in the TypeScript tag (example). Thanks. Commented Jan 29 at 10:40

0

You must log in to answer this question.

Browse other questions tagged .