Fix linkification for cropped URLs

Fixes #616.

Add many tests to make sure we cover various cases.

Author: nchevobbe

packages/devtools-reps/src/reps/rep-utils.js

@@ -5,6 +5,7 @@
 // Dependencies
 const validProtocols = /^(http|https|ftp|data|javascript|resource|chrome):/i;
 const tokenSplitRegex = /(\s|\'|\"|\\)+/;
+const ELLIPSIS = "\u2026";
 const dom = require("react-dom-factories");
 const { span } = dom;
 
@@ -138,11 +139,7 @@ function cropMultipleLines(text, limit) {
   return escapeNewLines(cropString(text, limit));
 }
 
-function rawCropString(text, limit, alternativeText) {
-  if (!alternativeText) {
-    alternativeText = "\u2026";
-  }
-
+function rawCropString(text, limit, alternativeText = ELLIPSIS) {
   // Crop the string only if a limit is actually specified.
   if (!limit || limit <= 0) {
     return text;
@@ -413,4 +410,5 @@ module.exports = {
   getGripPreviewItems,
   getGripType,
   tokenSplitRegex,
+  ELLIPSIS,
 };

packages/devtools-reps/src/reps/string.js

@@ -14,6 +14,7 @@ const {
   sanitizeString,
   wrapRender,
   tokenSplitRegex,
+  ELLIPSIS,
 } = require("./rep-utils");
 
 const dom = require("react-dom-factories");
@@ -62,47 +63,125 @@ function StringRep(props) {
     text = sanitizeString(text);
   }
 
-  if ((!member || !member.open) && cropLimit) {
-    text = rawCropString(text, cropLimit);
-  }
-
+  const shouldCrop = (!member || !member.open) && cropLimit;
   if (!containsURL(text)) {
+    if (shouldCrop) {
+      text = rawCropString(text, cropLimit);
+    }
     return span(config, text);
   }
 
-  const items = [];
+  return span(config,
+    ...getLinkifiedElements(text, shouldCrop && cropLimit, omitLinkHref, openLink));
+}
+
+/**
+ * Get an array of the elements representing the string, cropped if needed,
+ * with actual links.
+ *
+ * @param {String} text: The actual string to linkify.
+ * @param {Integer | null} cropLimit
+ * @param {Boolean} omitLinkHref: Do not create an href attribute if true.
+ * @param {Function} openLink: Function handling the link opening.
+ * @returns {Array<String|ReactElement>}
+ */
+function getLinkifiedElements(text, cropLimit, omitLinkHref, openLink) {
+  const halfLimit = Math.ceil((cropLimit - ELLIPSIS.length) / 2);
+  const startCropIndex = halfLimit;
+  const endCropIndex = text.length - halfLimit;
 
   // As we walk through the tokens of the source string, we make sure to preserve
   // the original whitespace that separated the tokens.
-  let tokens = text.split(tokenSplitRegex);
-  let textIndex = 0;
-  let tokenStart;
-  tokens.forEach((token, i) => {
-    tokenStart = text.indexOf(token, textIndex);
+  let currentIndex = 0;
+  const items = [];
+  for (let token of text.split(tokenSplitRegex)) {
     if (isURL(token)) {
-      items.push(text.slice(textIndex, tokenStart));
-      textIndex = tokenStart + token.length;
-
-      items.push(a({
-        className: "url",
-        title: token,
-        href: omitLinkHref === true
-          ? null
-          : token,
-        draggable: false,
-        onClick: openLink
-          ? e => {
-            e.preventDefault();
-            openLink(token);
-          }
-          : null
-      }, token));
+      // Let's grab all the non-url strings before the link.
+      const tokenStart = text.indexOf(token, currentIndex);
+      let nonUrlText = text.slice(currentIndex, tokenStart);
+      nonUrlText = getCroppedString(
+          nonUrlText, currentIndex, startCropIndex, endCropIndex);
+      if (nonUrlText) {
+        items.push(nonUrlText);
+      }
+
+      // Update the index to match the beginning of the token.
+      currentIndex = tokenStart;
+
+      let linkText = getCroppedString(token, currentIndex, startCropIndex, endCropIndex);
+      if (linkText) {
+        items.push(a({
+          className: "url",
+          title: token,
+          href: omitLinkHref === true
+            ? null
+            : token,
+          draggable: false,
+          onClick: openLink
+            ? e => {
+              e.preventDefault();
+              openLink(token);
+            }
+            : null
+        }, linkText));
+      }
+
+      currentIndex = tokenStart + token.length;
+    }
+  }
+
+  // Clean up any non-URL text at the end of the source string,
+  // i.e. not handled in the loop.
+  if (currentIndex !== text.length) {
+    let nonUrlText = text.slice(currentIndex, text.length);
+    if (currentIndex < endCropIndex) {
+      const cutIndex = endCropIndex - currentIndex;
+      nonUrlText = nonUrlText.substring(cutIndex);
     }
-  });
+    items.push(nonUrlText);
+  }
+
+  return items;
+}
+
+/**
+ * Returns a cropped substring given an offset, start and end crop indices in a parent
+ * string.
+ *
+ * @param {String} text: The substring to crop.
+ * @param {Integer} offset: The offset corresponding to the index at which the substring
+ *                          is in the parent string.
+ * @param {Integer} startCropIndex: the index where the start of the crop should happen
+ *                                  in the parent string
+ * @param {Integer} endCropIndex: the index where the end of the crop should happen
+ *                                  in the parent string
+ * @returns {String|null} The cropped substring, or null if the text is completly cropped.
+ */
+function getCroppedString(text, offset = 0, startCropIndex, endCropIndex) {
+  const start = offset;
+  const end = offset + text.length;
+
+  const shouldBeVisible = !(start >= startCropIndex && end <= endCropIndex);
+  if (!shouldBeVisible) {
+    return null;
+  }
+
+  const shouldCropEnd = start < startCropIndex && end > startCropIndex;
+  const shouldCropStart = start < endCropIndex && end > endCropIndex;
+  if (shouldCropEnd) {
+    const cutIndex = startCropIndex - start;
+    return text.substring(0, cutIndex) +
+      ELLIPSIS +
+      (shouldCropStart ? text.substring(endCropIndex - start) : "");
+  }
+
+  if (shouldCropStart) {
+    // The string should be cropped at the beginning.
+    const cutIndex = endCropIndex - start;
+    return text.substring(cutIndex);
+  }
 
-  // Clean up any non-URL text at the end of the source string.
-  items.push(text.slice(textIndex, text.length));
-  return span(config, ...items);
+  return text;
 }
 
 function supportsObject(object, noGrip = false) {

packages/devtools-reps/src/reps/tests/string-with-url.js

@@ -174,6 +174,163 @@ describe("test String with URL", () => {
     expect(openLink).toBeCalledWith(url2);
   });
 
+  it("renders multiple URLs with various spacing", () => {
+    const url1 = "http://example.com";
+    const url2 = "https://example.com/foo";
+    const string = `  ${url1}      ${url2}  ${url2}     ${url1}    `;
+    const element = renderRep(string, {useQuotes: false});
+    expect(element.text()).toEqual(string);
+    const links = element.find("a");
+    expect(links.length).toBe(4);
+  });
+
+  it("renders a cropped URL", () => {
+    const url = "http://example.com";
+    const openLink = jest.fn();
+    const element = renderRep(url, {
+      openLink,
+      useQuotes: false,
+      cropLimit: 15
+    });
+
+    expect(element.text()).toEqual("http://…ple.com");
+    const link = element.find("a");
+    expect(link.prop("href")).toBe(url);
+    expect(link.prop("title")).toBe(url);
+
+    link.simulate("click");
+    expect(openLink).toBeCalledWith(url);
+  });
+
+  it("renders URLs with a stripped string between", () => {
+    const text = "- http://example.fr --- http://example.us -";
+    const openLink = jest.fn();
+    const element = renderRep(text, {
+      openLink,
+      useQuotes: false,
+      cropLimit: 41
+    });
+
+    expect(element.text()).toEqual("- http://example.fr … http://example.us -");
+    const linkFr = element.find("a").at(0);
+    expect(linkFr.prop("href")).toBe("http://example.fr");
+    expect(linkFr.prop("title")).toBe("http://example.fr");
+
+    const linkUs = element.find("a").at(1);
+    expect(linkUs.prop("href")).toBe("http://example.us");
+    expect(linkUs.prop("title")).toBe("http://example.us");
+  });
+
+  it("renders URLs with a cropped string between", () => {
+    const text = "- http://example.fr ---- http://example.us -";
+    const openLink = jest.fn();
+    const element = renderRep(text, {
+      openLink,
+      useQuotes: false,
+      cropLimit: 42
+    });
+
+    expect(element.text()).toEqual("- http://example.fr -…- http://example.us -");
+    const linkFr = element.find("a").at(0);
+    expect(linkFr.prop("href")).toBe("http://example.fr");
+    expect(linkFr.prop("title")).toBe("http://example.fr");
+
+    const linkUs = element.find("a").at(1);
+    expect(linkUs.prop("href")).toBe("http://example.us");
+    expect(linkUs.prop("title")).toBe("http://example.us");
+  });
+
+  it("renders successive cropped URLs, one at the start, one at the end", () => {
+    const text = "- http://example-long.fr http://example.us -";
+    const openLink = jest.fn();
+    const element = renderRep(text, {
+      openLink,
+      useQuotes: false,
+      cropLimit: 20
+    });
+
+    expect(element.text()).toEqual("- http://e…ample.us -");
+    const linkFr = element.find("a").at(0);
+    expect(linkFr.prop("href")).toBe("http://example-long.fr");
+    expect(linkFr.prop("title")).toBe("http://example-long.fr");
+
+    const linkUs = element.find("a").at(1);
+    expect(linkUs.prop("href")).toBe("http://example.us");
+    expect(linkUs.prop("title")).toBe("http://example.us");
+  });
+
+  it("renders successive URLs, one cropped in the middle", () => {
+    const text = "- http://example-long.fr http://example.com http://example.us -";
+    const openLink = jest.fn();
+    const element = renderRep(text, {
+      openLink,
+      useQuotes: false,
+      cropLimit: 60
+    });
+
+    expect(element.text()).toEqual("- http://example-long.fr http:…xample.com http://example.us -");
+    const linkFr = element.find("a").at(0);
+    expect(linkFr.prop("href")).toBe("http://example-long.fr");
+    expect(linkFr.prop("title")).toBe("http://example-long.fr");
+
+    const linkCom = element.find("a").at(1);
+    expect(linkCom.prop("href")).toBe("http://example.com");
+    expect(linkCom.prop("title")).toBe("http://example.com");
+
+    const linkUs = element.find("a").at(2);
+    expect(linkUs.prop("href")).toBe("http://example.us");
+    expect(linkUs.prop("title")).toBe("http://example.us");
+  });
+
+  it("renders successive cropped URLs with cropped elements between", () => {
+    const text = "- http://example.fr test http://example.fr test http://example.us -";
+    const openLink = jest.fn();
+    const element = renderRep(text, {
+      openLink,
+      useQuotes: false,
+      cropLimit: 20
+    });
+
+    expect(element.text()).toEqual("- http://e…ample.us -");
+    const linkFr = element.find("a").at(0);
+    expect(linkFr.prop("href")).toBe("http://example.fr");
+    expect(linkFr.prop("title")).toBe("http://example.fr");
+
+    const linkUs = element.find("a").at(1);
+    expect(linkUs.prop("href")).toBe("http://example.us");
+    expect(linkUs.prop("title")).toBe("http://example.us");
+  });
+
+  it("renders a cropped URL followed by a cropped string", () => {
+    const text = "http://example.fr abcdefghijkl";
+    const openLink = jest.fn();
+    const element = renderRep(text, {
+      openLink,
+      useQuotes: false,
+      cropLimit: 20
+    });
+
+    expect(element.text()).toEqual("http://exa…cdefghijkl");
+    const linkFr = element.find("a").at(0);
+    expect(linkFr.prop("href")).toBe("http://example.fr");
+    expect(linkFr.prop("title")).toBe("http://example.fr");
+  });
+
+  it("renders a cropped string followed by a cropped URL", () => {
+    const text = "abcdefghijkl stripped http://example.fr ";
+    const openLink = jest.fn();
+    const element = renderRep(text, {
+      openLink,
+      useQuotes: false,
+      cropLimit: 20
+    });
+
+    expect(element.text()).toEqual("abcdefghij…xample.fr ");
+    const linkFr = element.find("a").at(0);
+    expect(linkFr.prop("href")).toBe("http://example.fr");
+    expect(linkFr.prop("title")).toBe("http://example.fr");
+  });
+
   it("does not render a link if the URL has no scheme", () => {
     const url = "example.com";
     const element = renderRep(url, {useQuotes: false});