Bug 1540069 - Remove openLink and pass isInContentPage to Rep. r=Honza.
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Tue, 23 Apr 2019 06:14:50 +0000
changeset 470465 113fa9baf3e0db62174708f2e97938356e0379ff
parent 470464 d6cdc1f35851af085218a5306a171599c92885fb
child 470466 cd1779b622d4ece63f8c6bf3e467eaa1c2c57ac4
push id35906
push useraciure@mozilla.com
push dateTue, 23 Apr 2019 22:14:56 +0000
treeherdermozilla-central@0ce3633f8b80 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersHonza
bugs1540069
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1540069 - Remove openLink and pass isInContentPage to Rep. r=Honza. This will allow links to be handled by the browser, so we don't have to have our own logic for that. The URLs in String rep are modified to have a target="_blank" attribute to match the current JSONViewer behaviour. Differential Revision: https://phabricator.services.mozilla.com/D26711
devtools/client/debugger/packages/devtools-reps/src/reps/string.js
devtools/client/jsonview/components/JsonPanel.js
devtools/client/jsonview/test/browser_jsonview_url_linkification.js
devtools/client/shared/components/reps/reps.js
--- a/devtools/client/debugger/packages/devtools-reps/src/reps/string.js
+++ b/devtools/client/debugger/packages/devtools-reps/src/reps/string.js
@@ -225,16 +225,17 @@ function getLinkifiedElements(text, crop
             className: "url",
             title: useUrl,
             draggable: false,
             // Because we don't want the link to be open in the current
             // panel's frame, we only render the href attribute if `openLink`
             // exists (so we can preventDefault) or if the reps will be
             // displayed in content page (e.g. in the JSONViewer).
             href: openLink || isInContentPage ? useUrl : null,
+            target: "_blank",
             onClick: openLink
               ? e => {
                   e.preventDefault();
                   openLink(useUrl, e);
                 }
               : null
           },
           linkText
--- a/devtools/client/jsonview/components/JsonPanel.js
+++ b/devtools/client/jsonview/components/JsonPanel.js
@@ -84,24 +84,17 @@ define(function(require, exports, module
       if (isObject(member.value) && member.hasChildren && member.open) {
         return null;
       }
 
       // Render the value (summary) using Reps library.
       return Rep(Object.assign({}, props, {
         cropLimit: 50,
         noGrip: true,
-        openLink(str) {
-          try {
-            const u = new URL(str);
-            if (u.protocol == "https:" || u.protocol == "http:") {
-              window.open(str, "_blank");
-            }
-          } catch (ex) { /* the link might be bust, then we do nothing */ }
-        },
+        isInContentPage: true,
       }));
     }
 
     renderTree() {
       // Append custom column for displaying values. This column
       // Take all available horizontal space.
       const columns = [{
         id: "value",
--- a/devtools/client/jsonview/test/browser_jsonview_url_linkification.js
+++ b/devtools/client/jsonview/test/browser_jsonview_url_linkification.js
@@ -53,26 +53,28 @@ add_task(async function() {
 async function testLinkNavigation({
   browser,
   url,
   urlText,
   clickLabel = false,
 }) {
   const onTabLoaded = BrowserTestUtils.waitForNewTab(gBrowser, url);
 
-  ContentTask.spawn(browser, [urlText || url, clickLabel], (args) => {
-    const [expectedURL, shouldClickLabel] = args;
+  ContentTask.spawn(browser, [urlText || url, url, clickLabel], (args) => {
+    const [expectedURLText, expectedURL, shouldClickLabel] = args;
     const {document} = content;
 
     if (shouldClickLabel === true) {
       document.querySelector(".jsonPanelBox .treeTable .treeLabel").click();
     }
 
     const link = document.querySelector(".jsonPanelBox .treeTable .treeValueCell a");
-    is(link.textContent, expectedURL, "The expected URL is displayed.");
+    is(link.textContent, expectedURLText, "The expected URL is displayed.");
+    is(link.href, expectedURL, "The URL was linkified.");
+
     link.click();
   });
 
   const newTab = await onTabLoaded;
   // We only need to check that newTab is truthy since
   // BrowserTestUtils.waitForNewTab checks the URL.
   ok(newTab, "The expected tab was opened.");
 }
--- a/devtools/client/shared/components/reps/reps.js
+++ b/devtools/client/shared/components/reps/reps.js
@@ -4565,16 +4565,17 @@ function getLinkifiedElements(text, crop
         className: "url",
         title: useUrl,
         draggable: false,
         // Because we don't want the link to be open in the current
         // panel's frame, we only render the href attribute if `openLink`
         // exists (so we can preventDefault) or if the reps will be
         // displayed in content page (e.g. in the JSONViewer).
         href: openLink || isInContentPage ? useUrl : null,
+        target: "_blank",
         onClick: openLink ? e => {
           e.preventDefault();
           openLink(useUrl, e);
         } : null
       }, linkText));
     }
 
     currentIndex = currentIndex + useUrl.length;