GoogleChromeLabs / link-to-text-fragment

Browser extension that allows for linking to arbitrary text fragments.
https://chrome.google.com/webstore/detail/link-to-text-fragment/pbcodcjpfjdpcineamnnmbkkmkdpajjg
Apache License 2.0
412 stars 33 forks source link

Incompatible encoding (`-`, `&`, `,`) #87

Closed adamalfredsson closed 1 year ago

adamalfredsson commented 1 year ago

It seems like this is using encodeURIComponent() for the selected text, which is insufficient as it does not encode -, , and &.

https://github.com/GoogleChromeLabs/link-to-text-fragment/blob/d9bc479bfac476fcde111af712f98d7d33815be0/content_script.js#LL42C6-L42C6

The WICG specification states:

Dash (-), ampersand (&), and comma (,) characters in text snippets must be percent-encoded to avoid being interpreted as part of the text fragment syntax.

Suggested solution:

function encodeCharacter(char: string) {
  switch (char) {
    case ",":
      return "%2C";
    case "-":
      return "%2D";
    case "&":
      return "%26";
    default:
      return encodeURIComponent(char);
  }
}

// https://github.com/WICG/scroll-to-text-fragment#identifying-a-text-snippet
function encodeTextFragment(text: string) {
  return [",", "-", "&"].reduce(
    (encoded, char) => encoded.replaceAll(char, encodeCharacter(char)),
    encodeURIComponent(text)
  );
}

Related issues:

tomayac commented 1 year ago

Closing this in favor of https://github.com/GoogleChromeLabs/text-fragments-polyfill/issues/115, which is the code the extension uses for generating URLs.