Bug 1444327 - Bring back ability to see and copy font URLs; r=jdescottes draft
authorPatrick Brosset <pbrosset@mozilla.com>
Wed, 14 Mar 2018 16:16:55 +0100
changeset 769242 ac406d91c78e5222854b53f245ad2d93428bfc4d
parent 769198 0b997836c7cf258a2b821eaa1c1ee66b9289ab17
push id103084
push userbmo:pbrosset@mozilla.com
push dateMon, 19 Mar 2018 08:55:15 +0000
reviewersjdescottes
bugs1444327, 1437548, 1442001
milestone61.0a1
Bug 1444327 - Bring back ability to see and copy font URLs; r=jdescottes In the fonts panel UI prior to Firefox 60, remote font URLs used to be displayed in full in a text input field. It made it easy to copy them. With the redesign that happened in 60 (bug 1437548 and 1442001), getting the URL became harder. The URL isn't visible anymore easily. There's a link that can be clicked to load the URL in the browser, or it can also be copied from the @font-face CSS rule code section. But that's harder. This change adds the beginning of the URL back (with an ellipsis) and a simple button that copies the link. Note that the new test failed intermittently on non e10s (took too long). This was because of a react middleware which was logging all actions, which, in non-e10s, ended up logging StyleRuleActors, which got serialized and caused way too much logs to be printed, slowing the test down. So the test was disabled on non-e10s. MozReview-Commit-ID: 2oSMoWKYhTk
devtools/client/inspector/fonts/components/Font.js
devtools/client/inspector/fonts/test/browser.ini
devtools/client/inspector/fonts/test/browser_fontinspector.js
devtools/client/inspector/fonts/test/browser_fontinspector_copy-URL.js
devtools/client/jar.mn
devtools/client/locales/en-US/font-inspector.properties
devtools/client/themes/fonts.css
devtools/client/themes/images/copy.svg
--- a/devtools/client/inspector/fonts/components/Font.js
+++ b/devtools/client/inspector/fonts/components/Font.js
@@ -5,16 +5,18 @@
 "use strict";
 
 const { createFactory, PureComponent } = require("devtools/client/shared/vendor/react");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 
 const FontPreview = createFactory(require("./FontPreview"));
 
+loader.lazyRequireGetter(this, "clipboardHelper", "devtools/shared/platform/clipboard");
+
 const { getStr } = require("../utils/l10n");
 const Types = require("../types");
 
 class Font extends PureComponent {
   static get propTypes() {
     return {
       font: PropTypes.shape(Types.font).isRequired,
       fontOptions: PropTypes.shape(Types.fontOptions).isRequired,
@@ -25,28 +27,33 @@ class Font extends PureComponent {
   constructor(props) {
     super(props);
 
     this.state = {
       isFontFaceRuleExpanded: false,
     };
 
     this.onFontFaceRuleToggle = this.onFontFaceRuleToggle.bind(this);
+    this.onCopyURL = this.onCopyURL.bind(this);
   }
 
   componentWillReceiveProps(newProps) {
     if (this.props.font.name === newProps.font.name) {
       return;
     }
 
     this.setState({
       isFontFaceRuleExpanded: false,
     });
   }
 
+  onCopyURL() {
+    clipboardHelper.copyString(this.props.font.URI);
+  }
+
   onFontFaceRuleToggle(event) {
     this.setState({
       isFontFaceRuleExpanded: !this.state.isFontFaceRuleExpanded
     });
     event.stopPropagation();
   }
 
   renderFontCSSCode(rule, ruleText) {
@@ -76,37 +83,43 @@ class Font extends PureComponent {
             className: "font-css-code-expander",
             onClick: this.onFontFaceRuleToggle,
           }
         ),
       trailing
     );
   }
 
-  renderFontTypeAndURL(url, format) {
+  renderFontOrigin(url) {
     if (!url) {
       return dom.p(
         {
-          className: "font-format-url"
+          className: "font-origin system"
         },
         getStr("fontinspector.system")
       );
     }
 
     return dom.p(
       {
-        className: "font-format-url"
+        className: "font-origin remote",
       },
-      getStr("fontinspector.remote"),
-      dom.a(
+      dom.span(
         {
-          className: "font-url",
-          href: url
+          className: "url",
+          title: url
         },
-        format
+        url
+      ),
+      dom.button(
+        {
+          className: "copy-icon",
+          onClick: this.onCopyURL,
+          title: getStr("fontinspector.copyURL"),
+        }
       )
     );
   }
 
   renderFontName(name) {
     return dom.h1(
       {
         className: "font-name"
@@ -134,29 +147,28 @@ class Font extends PureComponent {
       font,
       fontOptions,
       onPreviewFonts,
     } = this.props;
 
     let { previewText } = fontOptions;
 
     let {
-      format,
       name,
       previewUrl,
       rule,
       ruleText,
       URI,
     } = font;
 
     return dom.li(
       {
         className: "font",
       },
       this.renderFontName(name),
       FontPreview({ previewText, previewUrl, onPreviewFonts }),
-      this.renderFontTypeAndURL(URI, format),
+      this.renderFontOrigin(URI),
       this.renderFontCSSCode(rule, ruleText)
     );
   }
 }
 
 module.exports = Font;
--- a/devtools/client/inspector/fonts/test/browser.ini
+++ b/devtools/client/inspector/fonts/test/browser.ini
@@ -10,13 +10,16 @@ support-files =
   !/devtools/client/commandline/test/helpers.js
   !/devtools/client/inspector/test/head.js
   !/devtools/client/inspector/test/shared-head.js
   !/devtools/client/shared/test/shared-head.js
   !/devtools/client/shared/test/test-actor.js
   !/devtools/client/shared/test/test-actor-registry.js
 
 [browser_fontinspector.js]
+[browser_fontinspector_copy-URL.js]
+skip-if = !e10s # too slow on !e10s, logging fully serialized actors (Bug 1446595)
+subsuite = clipboard
 [browser_fontinspector_edit-previews.js]
 [browser_fontinspector_expand-css-code.js]
 [browser_fontinspector_other-fonts.js]
 [browser_fontinspector_text-node.js]
 [browser_fontinspector_theme-change.js]
--- a/devtools/client/inspector/fonts/test/browser_fontinspector.js
+++ b/devtools/client/inspector/fonts/test/browser_fontinspector.js
@@ -5,73 +5,59 @@
 
 requestLongerTimeout(2);
 
 const TEST_URI = URL_ROOT + "browser_fontinspector.html";
 const FONTS = [{
   name: "Ostrich Sans Medium",
   remote: true,
   url: URL_ROOT + "ostrich-regular.ttf",
-  format: "truetype",
   cssName: "bar"
 }, {
   name: "Ostrich Sans Black",
   remote: true,
   url: URL_ROOT + "ostrich-black.ttf",
-  format: "",
   cssName: "bar"
 }, {
   name: "Ostrich Sans Black",
   remote: true,
   url: URL_ROOT + "ostrich-black.ttf",
-  format: "",
   cssName: "bar"
 }, {
   name: "Ostrich Sans Medium",
   remote: true,
   url: URL_ROOT + "ostrich-regular.ttf",
-  format: "",
   cssName: "barnormal"
 }];
 
 add_task(function* () {
   let { inspector, view } = yield openFontInspectorForURL(TEST_URI);
   ok(!!view, "Font inspector document is alive.");
 
   let viewDoc = view.document;
 
   yield testBodyFonts(inspector, viewDoc);
   yield testDivFonts(inspector, viewDoc);
 });
 
 function isRemote(fontLi) {
-  return fontLi.querySelectorAll(".font-format-url a").length === 1;
-}
-
-function getFormat(fontLi) {
-  let link = fontLi.querySelector(".font-format-url a");
-  if (!link) {
-    return null;
-  }
-
-  return link.textContent;
+  return fontLi.querySelector(".font-origin").classList.contains("remote");
 }
 
 function* testBodyFonts(inspector, viewDoc) {
   let lis = getUsedFontsEls(viewDoc);
   is(lis.length, 5, "Found 5 fonts");
 
   for (let i = 0; i < FONTS.length; i++) {
     let li = lis[i];
     let font = FONTS[i];
 
-    is(getName(li), font.name, "font " + i + " right font name");
-    is(isRemote(li), font.remote, "font " + i + " remote value correct");
-    is(li.querySelector(".font-url").href, font.url, "font " + i + " url correct");
-    is(getFormat(li), font.format, "font " + i + " format correct");
+    is(getName(li), font.name, `font ${i} right font name`);
+    is(isRemote(li), font.remote, `font ${i} remote value correct`);
+    is(li.querySelector(".font-origin").textContent, font.url, `font ${i} url correct`);
   }
 
   // test that the bold and regular fonts have different previews
   let regSrc = lis[0].querySelector(".font-preview").src;
   let boldSrc = lis[1].querySelector(".font-preview").src;
   isnot(regSrc, boldSrc, "preview for bold font is different from regular");
 
   // test system font
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/fonts/test/browser_fontinspector_copy-URL.js
@@ -0,0 +1,25 @@
+/* vim: set ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test that an icon appears next to web font URLs, and that clicking it copies the URL
+// to the clipboard thanks to it.
+
+const TEST_URI = URL_ROOT + "browser_fontinspector.html";
+
+add_task(async function() {
+  let { view } = await openFontInspectorForURL(TEST_URI);
+  let viewDoc = view.document;
+
+  let fontEl = getUsedFontsEls(viewDoc)[0];
+  let linkEl = fontEl.querySelector(".font-origin");
+  let iconEl = linkEl.querySelector(".copy-icon");
+
+  ok(iconEl, "The icon is displayed");
+  is(iconEl.getAttribute("title"), "Copy URL", "This is the right icon");
+
+  info("Clicking the button and waiting for the clipboard to receive the URL");
+  await waitForClipboardPromise(() => iconEl.click(), linkEl.textContent);
+});
--- a/devtools/client/jar.mn
+++ b/devtools/client/jar.mn
@@ -118,16 +118,17 @@ devtools.jar:
     skin/images/breadcrumbs-divider.svg (themes/images/breadcrumbs-divider.svg)
     skin/images/filters.svg (themes/images/filters.svg)
     skin/images/filter-swatch.svg (themes/images/filter-swatch.svg)
     skin/images/grid.svg (themes/images/grid.svg)
     skin/images/angle-swatch.svg (themes/images/angle-swatch.svg)
     skin/images/pseudo-class.svg (themes/images/pseudo-class.svg)
     skin/images/controls.png (themes/images/controls.png)
     skin/images/controls@2x.png (themes/images/controls@2x.png)
+    skin/images/copy.svg (themes/images/copy.svg)
     skin/images/animation-fast-track.svg (themes/images/animation-fast-track.svg)
     skin/images/performance-details-waterfall.svg (themes/images/performance-details-waterfall.svg)
     skin/images/performance-details-call-tree.svg (themes/images/performance-details-call-tree.svg)
     skin/images/performance-details-flamegraph.svg (themes/images/performance-details-flamegraph.svg)
     skin/breadcrumbs.css (themes/breadcrumbs.css)
     skin/chart.css (themes/chart.css)
     skin/widgets.css (themes/widgets.css)
     skin/images/power.svg (themes/images/power.svg)
--- a/devtools/client/locales/en-US/font-inspector.properties
+++ b/devtools/client/locales/en-US/font-inspector.properties
@@ -4,24 +4,25 @@
 
 # LOCALIZATION NOTE This file contains the Font Inspector strings.
 # The Font Inspector is a panel accessible in the Inspector sidebar.
 
 # LOCALIZATION NOTE (fontinspector.system) This label indicates that the font is a local
 # system font.
 fontinspector.system=system
 
-# LOCALIZATION NOTE (fontinspector.remote) This label indicates that the font is a remote
-# font.
-fontinspector.remote=remote
-
 # LOCALIZATION NOTE (fontinspector.noFontsOnSelectedElement): This label is shown when
 # no fonts found on the selected element.
 fontinspector.noFontsOnSelectedElement=No fonts were found for the current element.
 
 # LOCALIZATION NOTE (fontinspector.otherFontsInPageHeader): This is the text for the
 # header of a collapsible section containing other fonts used in the page.
 fontinspector.otherFontsInPageHeader=Other fonts in page
 
 # LOCALIZATION NOTE (fontinspector.editPreview): This is the text that appears in a
 # tooltip on hover of a font preview string. Clicking on the string opens a text input
 # where users can type to change the preview text.
 fontinspector.editPreview=Click to edit preview
+
+# LOCALIZATION NOTE (fontinspector.copyURL): This is the text that appears in a tooltip
+# displayed when the user hovers over the copy icon next to the font URL.
+# Clicking the copy icon copies the full font URL to the user's clipboard
+fontinspector.copyURL=Copy URL
--- a/devtools/client/themes/fonts.css
+++ b/devtools/client/themes/fonts.css
@@ -21,16 +21,17 @@
   list-style: none;
 }
 
 .font {
   border: 1px solid var(--theme-splitter-color);
   border-width: 0 1px 1px 0;
   display: grid;
   grid-template-columns: 1fr auto;
+  grid-column-gap: 10px;
   padding: 10px 20px;
 }
 
 #font-container .theme-twisty {
   display: inline-block;
   cursor: pointer;
   vertical-align: bottom;
 }
@@ -103,40 +104,49 @@
   color: var(--theme-body-color-inactive);
   border-radius: 3px;
   border-style: solid;
   border-width: 1px;
   text-align: center;
   vertical-align: middle;
 }
 
-.font-format-url {
-  text-transform: capitalize;
+.font-origin {
   margin-top: .2em;
   color: var(--grey-50);
+  justify-self: start;
+}
+
+.font-origin.system {
+  text-transform: capitalize;
+}
+
+.font-origin.remote {
+  display: grid;
+  grid-template-columns: 1fr 20px;
 }
 
-.font-url {
-  margin-inline-start: .5em;
-  text-decoration: underline;
-  color: var(--theme-highlight-blue);
-  background: transparent;
-  border: none;
-  cursor: pointer;
+.font-origin.remote .url {
+  text-overflow: ellipsis;
+  overflow: hidden;
+  white-space: nowrap;
 }
 
-.font-url::after {
-  content: "";
-  display: inline-block;
-  height: 13px;
-  width: 13px;
-  margin: -.3rem .15rem 0 0.25rem;
-  vertical-align: middle;
-  background-image: url(chrome://devtools-shim/content/aboutdevtools/images/external-link.svg);
-  background-repeat: no-repeat;
-  background-size: 13px 13px;
+.font-origin .copy-icon {
+  border: 0;
+  padding: 0;
+  position: relative;
+  cursor: pointer;
+  width: 12px;
+  height: 12px;
+  place-self: center;
+
+  background: url(chrome://devtools/skin/images/copy.svg) no-repeat;
+  background-size: 12px;
+  background-position-x: -1px;
   -moz-context-properties: fill;
-  fill: var(--blue-60);
+  fill: var(--theme-toolbar-color);
+
 }
 
 #font-container .devtools-sidepanel-no-result + .accordion {
   border-block-start: 1px solid var(--theme-splitter-color);
 }
new file mode 100644
--- /dev/null
+++ b/devtools/client/themes/images/copy.svg
@@ -0,0 +1,6 @@
+<!-- This Source Code Form is subject to the terms of the Mozilla Public
+   - License, v. 2.0. If a copy of the MPL was not distributed with this
+   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 12 12" fill="context-fill #0b0b0b">
+  <path d="M5.70001221,11.125 L5.70001221,4.625 L7.66001225,4.625 L7.66001225,6.94642857 C7.66001225,7.20271429 7.87953225,7.41071429 8.15001225,7.41071429 L10.6000123,7.41071429 L10.6000123,11.125 L5.70001221,11.125 Z M4.7,5.475 L4.7,7.275 L2,7.275 L2,0.975 L3.8,0.975 L3.8,3.225 C3.8,3.4734 4.0016,3.675 4.25,3.675 L6.5,3.675 C5.5064,3.675 4.7,4.4814 4.7,5.475 Z M6.84002447,3.00050006 L4.65002441,3.00050006 L4.65002441,0.8105 L6.84002447,3.00050006 Z M10.5,6.6000061 L8.5,6.6000061 L8.5,4.6000061 L10.5,6.6000061 Z M11.28025,6.21975 L9.03025,3.96975 C8.89,3.82875 8.6995,3.75 8.5,3.75 L7.75,3.75 L7.75,3 C7.75,2.80125 7.67125,2.61 7.53025,2.46975 L5.28025,0.21975 C5.14,0.07875 4.94875,0 4.75,0 L2.5,0 C1.672,0 1,0.672 1,1.5 L1,6.75 C1,7.578 1.672,8.25 2.5,8.25 L4.75,8.25 L4.75,10.5 C4.75,11.328 5.422,12 6.25,12 L10,12 C10.828,12 11.5,11.328 11.5,10.5 L11.5,6.75 C11.5,6.55125 11.42125,6.36 11.28025,6.21975 Z"></path>
+</svg>