697983 - Implement a Font Inspector (built in)

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Paul Rouget [:paul]]

It would be interesting to get a font-dedicated tool. The scope of this tool needs to be defined. The tool doesn't have to be part of the built-in DevTools, and could be based on the DevTools jetpack module (see bug 697982).

Comment 1

[Paul Rouget [:paul]]

Code that can help:
https://github.com/paulrouget/font-inspector/

Comment 2

[Paul Rouget [:paul]]

Attachment: Mockup by @brampitoyo

Comment 3

[Paul Rouget [:paul]]

Attachment: Mockups by @ivanovpavel and @Ja33ko

Comment 4

[Paul Rouget [:paul]]

@brampitoyo, any comment on Ivanov's designs?

Comment 5

[Bram Pitoyo [:bram]]

@paul,

#3 seems to me like the one that’s most logically organized. That is, it fulfills two purposes at once:

1. It allows me to get a “typographic overview” of the element. Rather than presenting a list, it organizes properties into groups.

From left to right and top to bottom, these groups are:
a) Specimen and URL
b) Font-family and current selection
c) Common tweaks: size, leading, style and weight
d) Color: allotted its own space because of the many ways that you can code it (RGB, Hex, name, etc.)

This fulfills one user scenario, which is: “I want to quickly see the name, size and color of this font”.


2. At the same time, it also allows me to tweak very specific things about the element quickly.
This covers many scenarios:
a) “How far can I copyfit this headline without changing its size?”: tweak letter-spacing & word-spacing
b) “Would this sidebar read better if I make it right-aligned?”: tweak alignment
c) “Would this text still look good if I make it bold?”: tweak weight
etc.

Design #3 fulfills both: if user wants to just see the font’s setting and then move on, she can do it. But if she wants to tweak one specific property, she can also do it with not very many clicks”.


FEEDBACKS

1. Organizing by CSS wording (font-something, text-something, spacing-related, etc.) makes sense logically, but isn’t necessarily intuitive. For instance, people often associate bold, italic and underline properties together, but bold and italic is under font, and underline is under text. It’s better if we organize it by property use: which one is the most tweaked? Which ones are often tweaked together? Which ones do similar things?

Ivanov had done a great job with organizing the “Basic” mode, so here’s a sample of how things could be organized in “Detail”:
a) Common tweaks: size, line-height, bold, italic, underline
b) Color-related: color, opacity, shadow
c) Spacing-related: Align, Indent
d) Character-related: transform, variant, stretch, size-adjust

2. Should the implementation of Basic and Detail navigation be aligned with what the Devtools team is already doing? Is it a down arrow button, a link text, a tab?


SIDEBAR

I think the ability to “compare and contrast”, having an edit mode and a “Copy as CSS” button are good. This is because web developers might not have the same typography practice that print designers have had.

In the past, many print designers are used to selecting font, size and style before setting things on a page: “tweak now, type later”. Since word processing came about, users are increasingly used to “type now, tweak later”. I think the font inspector should be built with the fact that developers are used to tweak type *after* the text is already set.

Comment 6

[Paul Rouget [:paul]]

(In reply to Bram Pitoyo from comment #5)
> 2. Should the implementation of Basic and Detail navigation be aligned with
> what the Devtools team is already doing? Is it a down arrow button, a link
> text, a tab?

We don't have such UI pattern yet ("basic to details").

Comment 7

[Paul Rouget [:paul]]

I can see 3 steps in the development of this tool:
1. Basic overview
2. Details view
3. Edit mode

Can we try to define what "Step 1" should be?

(I would also like to have Jérémie's thoughts on this)

Comment 8

[Bram Pitoyo [:bram]]

Attachment: Mockup of navigation

Comment 9

[Bram Pitoyo [:bram]]

Attachment: Mockup of CSS properties organization schemes

Comment 10

[Paul Rouget [:paul]]

(In reply to Bram Pitoyo from comment #8)
> Created attachment 571033 [details]
> Mockup of navigation

The expand link looks like the right way to do it, and will probably feet better in the devtools sidebar (see bug 695440).

Comment 11

[Bram Pitoyo [:bram]]

(In reply to Paul Rouget [:paul] from comment #6)
> We don't have such UI pattern yet ("basic to details").

I’ve uploaded “basic to details” UI patterns under the file “Mockup of navigation”. Along with the original that Ivanov had designed (tab), we also have expand link and button-based.


(In reply to Paul Rouget [:paul] from comment #7)
> I can see 3 steps in the development of this tool […]
> Can we try to define what "Step 1" should be?

1. Finalize the properties we want under Basic view
2. Mockup different ways of organizing properties, select a few
3. Make quick implementations, then do A-B testing
4. Refine, then deploy a “final” Basic view

So far, both of our mockups had included 5 items on the Basic view: size, line-height, style, weight and color.

It would really help if we get some opinions on this. Do people think that any property should or should not be on this list?

Comment 12

[Kosta]

Hi Guys,

The mock ups that Bram prepared are great and really illustrate most of the options that we have covered so far. 

We've played around the basic/detailed view sample and I think that the expand link is the best pick here - the only thing that bothers me is the behavior of the tool when the user inspects something in the bottom of his/hers browser window. That said the tool window is not going to have a fixed position in the browser window right - meaning it will appear wherever the user inspects text - similar to the WhatFont tool?

On the other hand in case we have the fixed 300x300 window to play in I think that the best option would be the "By Name General" inner tabbed option. Bram got our #4 idea great in his mock up and the only thing that I think will be better is if we stick to the fixed 300px height of the window. 
The properties that we though of might be organized like:

General - Font name, @font-face url (if any), Font-size, Line-height, Font-style, Font-weight, color;
Font - all the font properties;
Text - all the text properties;
Other - letter-spacing, word-spacing, line-height, opacity;

The order here is slightly different than the one Bram proposed and I think we can mock-try both ways and see which one works better. The general/basic view elements are most important here.  

Pavel and I though that it might be nice to have an option to enable/disable some inherited properties similar to Firebug - we have not really though of this one deeper though since there might be some confusions here. What do you guys think?

We loved the option to have the edited css copied to your clipboard as well -  we will think of a good way to expose this one in the window.

Anyway what we thought of is that it will best to have a working mock up that we can actually play with. So share your thoughts on what do you think about the tool window size, position and behavior and will prepare the html mock up.

Comment 13

[Jeremie Patonnier :Jeremie]

(In reply to Paul Rouget [:paul] from comment #7)
> (I would also like to have Jérémie's thoughts on this)

Hi,

At Paul's request, here are some thought about that Font Inspector.

First of all, I have to said that I like the idea of a progressive Inspector with a general view and a detailed view. Indeed, there is so many information about fonts that it's important to have a quick overview and the opportunity to dig in.

In my opinion, the most important information when inspecting an elements are :

- The font family in use
- The font size (with the ability to switch between units, at least px, em and rem... yes, I know, em is tricky)
- The font variants in use (bold, italic, caps, small caps and ligatures)

When going into detail, there are 4 levels of information that are relevant :

1. Information about the font style (micro-typography level)

Those are related to the following CSS properties : font-family, font-size, font-size-adjust, font-stretch, font-style, font-variant, font-weight, text-transform, -moz-font-feature-settings and the upcoming CSS3 properties font-variant-* (if any)

IMHO, those information are the most important and should all be displayed in the "Quick view" (except for font-size-adjust and font-stretch which are quite uncommon)

2. Information about the text style (macro-typography level)

Those are related to the following CSS properties : direction, hyphens, letter-spacing, line-height, text-align, text-decoration, text-decoration-color, text-decoration-line, text-decoration-style, text-indent, text-overflow, text-shadow, unicode-bidi, vertical-align, white-space, word-spacing, word-wrap

3. Information about the font file (glyphs level)

Having the opportunity to inspect font style is good, but having the opportunity to dig into the font file itself would be awesome (^_^). As an author, I wish to have the opportunity to check which glyph is available inside the font file. This is important because it's easy to build font files with a small subset of glyph (for performance reason) and sometime it's good to be able to check for the availability of a specific glyph while you are inspecting your font style (this is especially true when you are debugging the rendering of a contributed content).

It will also be good to have the opportunity to read the font's metadatas, for example to know the source of the font, it's author, it's license (very important), etc.

4. some less interesting things

Those are related to uncommon CSS rules such as : marks, orphans, outline-color, outline-style, outline-width, outline-offset, quotes, text-rendering

Finally, I wish to raise an issue about colors. In HTML, it's easy to determine the color of a text, it's basically done through the CSS "color" property. However, it becomes a bit trickier with SVG. SVG text can be filled with color but also with patterns or gradients through the CSS "fill", "fill-*", "stroke" and "stroke-*" properties. Because, due to HTML5, it's possible to have SVG inside HTML, it's possible to have such problematic color and it's not obvious to make it understandable for a user.

Next time, I will try to dig into the mokeups to help improved them :)

Comment 14

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Does the font inspector will let you know which font has been asked for an element and which font is really used by the platform?
It could be helpful for pdf.js for example.

Comment 15

[Paul Rouget [:paul]]

(In reply to Vivien Nicolas (:vingtetun) from comment #14)
> Does the font inspector will let you know which font has been asked for an
> element and which font is really used by the platform?

Yes.

Comment 16

[Bram Pitoyo [:bram]]

I want to point out that Jonathan Kew (jkew) had worked on the fontinfo add-on for Firefox, and his work there might help us design the font inspector: https://addons.mozilla.org/en-US/firefox/addon/fontinfo/

Comment 17

[Christian Sonne [:cers]]

I've created a version of Paul's font-inspector extension that looks like option 3 from comment #3 https://github.com/cers/font-inspector

It's not pixel perfect (yet), but it's reasonably close.

Comment 18

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Christian Sonne - here is the font-size info of comment #3
http://pivanov.com/font-inspector/font-inspector-sizes.png

If I can help you with something(html/css) just ping me.

Comment 19

[Paul Rouget [:paul]]

Cers and Pavel, thank you for your work!

I think the font-inspector should live in the sidebar (we have "Rules" and "Computed", we should have "Font" too). Also, if we want to get a coherent look, we should re-think the current design to make it more integrated with the sidebar tools. See what we are doing there: bug 712469 - take a look at this mockup too: attachment 604219 [details]

But, the way the sidebar works today can be a problem. We need a proper API to add a tool in the sidebar. This should be addressed by bug 707809

Comment 20

[Paul Rouget [:paul]]

I've updated my addon for the toolbox:
- screenshot: http://i.imgur.com/3OWmDSk.png
- https://addons.mozilla.org/en-US/firefox/addon/font-inspector/
- https://github.com/paulrouget/font-inspector/

Comment 21

[Victor Porof [:vporof][:vp]]

(In reply to Paul Rouget [:paul] from comment #20)
> I've updated my addon for the toolbox:
> - screenshot: http://i.imgur.com/3OWmDSk.png
> - https://addons.mozilla.org/en-US/firefox/addon/font-inspector/
> - https://github.com/paulrouget/font-inspector/

That looks amazing.

Comment 22

[Paul Rouget [:paul]]

Attachment: patch v1

This should be built in.
It's now part of the inspector (not in its own panel).

Comment 23

[Paul Rouget [:paul]]

built in. Because we're cool.

Comment 24

[Paul Rouget [:paul]]

Maybe embedding a 200Kb font in the test directory was not a great idea :)

Comment 25

[Paul Rouget [:paul]]

Attachment: screenshot

Comment 26

[Joe Walker [:jwalker]]

Comment on attachment 719994 [details] [diff] [review]
patch v1

Review of attachment 719994 [details] [diff] [review]:
-----------------------------------------------------------------

Reviewed all but JS. Will have to come back to that.

::: browser/app/profile/firefox.js
@@ +1146,5 @@
>  //   indenting and bracket recognition.
>  pref("devtools.editor.component", "orion");
>  
> +// Enable the Font Inspector
> +pref("devtools.fontinspector.enabled", true);

Maybe land preffed off to start off with?

::: browser/devtools/Makefile.in
@@ +27,5 @@
>    shared \
>    responsivedesign \
>    framework \
>    profiler \
> +  font-inspector \

Could we have fontinspector? Our other dirs are all lower, no word separators, and I believe that some module loaders require this (although maybe nothing that we're likely to use)

::: browser/devtools/layoutview/view.js
@@ +103,5 @@
>      if (this.browser) {
>        this.browser.removeEventListener("MozAfterPaint", this.update, true);
>      }
>      if (this.inspector.highlighter) {
> +      this.inspector.highlighter.off("locked", this.onHighlighterLocked);

This looks like a memory leak? If we don't land the font inspector soon then we should find another way to land this.

::: browser/themes/osx/devtools/font-inspector.css
@@ +3,5 @@
> +}
> +
> +body {
> +  background: #F9F9F9;
> +  font-family: Arial;

Arial. On OSX. In a font inspector. Isn't that some kind of heresy. Maybe go the full monty and use Comic Sans? :)

@@ +48,5 @@
> +  display: block;
> +}
> +
> +.font-name {
> +  display: inline;

Maybe this should be content css?

::: browser/themes/windows/devtools/font-inspector.css
@@ +1,1 @@
> +* {

Thinking out loud: Some time ago people would freak out at * at the end of a selector. This isn't browser css, and maybe css engines are fast enough to not worry any more.

@@ +43,5 @@
> +}
> +
> +.font-info {
> +  font-size: 1rem;
> +  /*min-height: 100px;*/

Probably remove?

Comment 27

[Paul Rouget [:paul]]

(In reply to Joe Walker [:jwalker] from comment #26)
> Reviewed all but JS. Will have to come back to that.

Ok.

> Maybe land preffed off to start off with?

Can we only pref it off in Aurora or Beta?

> ::: browser/devtools/Makefile.in
> @@ +27,5 @@
> >    shared \
> >    responsivedesign \
> >    framework \
> >    profiler \
> > +  font-inspector \
> 
> Could we have fontinspector? Our other dirs are all lower, no word
> separators, and I believe that some module loaders require this (although
> maybe nothing that we're likely to use)

Ok.

> ::: browser/devtools/layoutview/view.js
> @@ +103,5 @@
> >      if (this.browser) {
> >        this.browser.removeEventListener("MozAfterPaint", this.update, true);
> >      }
> >      if (this.inspector.highlighter) {
> > +      this.inspector.highlighter.off("locked", this.onHighlighterLocked);
> 
> This looks like a memory leak? If we don't land the font inspector soon then
> we should find another way to land this.

I'll file a bug if I see we're struggling with this patch.

> 
> ::: browser/themes/osx/devtools/font-inspector.css
> @@ +3,5 @@
> > +}
> > +
> > +body {
> > +  background: #F9F9F9;
> > +  font-family: Arial;
> 
> Arial. On OSX. In a font inspector. Isn't that some kind of heresy. Maybe go
> the full monty and use Comic Sans? :)

I didn't know Arial was bad. What should I use then?

> @@ +48,5 @@
> > +  display: block;
> > +}
> > +
> > +.font-name {
> > +  display: inline;
> 
> Maybe this should be content css?

This is a styling thing (not functional).

> ::: browser/themes/windows/devtools/font-inspector.css
> @@ +1,1 @@
> > +* {
> 
> Thinking out loud: Some time ago people would freak out at * at the end of a
> selector. This isn't browser css, and maybe css engines are fast enough to
> not worry any more.

Universal rules are usually bad. But here it's pretty harmless (very small DOM).

> @@ +43,5 @@
> > +}
> > +
> > +.font-info {
> > +  font-size: 1rem;
> > +  /*min-height: 100px;*/
> 
> Probably remove?

Yep.

Comment 28

[Paul Rouget [:paul]]

> > ::: browser/themes/osx/devtools/font-inspector.css
> > @@ +3,5 @@
> > > +}
> > > +
> > > +body {
> > > +  background: #F9F9F9;
> > > +  font-family: Arial;
> > 
> > Arial. On OSX. In a font inspector. Isn't that some kind of heresy. Maybe go
> > the full monty and use Comic Sans? :)
> 
> I didn't know Arial was bad. What should I use then?

I'll use the default font (I won't specify any).

Comment 29

[Paul Rouget [:paul]]

Attachment: patch v1.1

(this will also require a security review, because I'm injecting page content into an iframe located in a chrome iframe)

Comment 30

[Joe Walker [:jwalker]]

(In reply to Paul Rouget [:paul] from comment #27)
> (In reply to Joe Walker [:jwalker] from comment #26)
> > Reviewed all but JS. Will have to come back to that.
> 
> Ok.
> 
> > Maybe land preffed off to start off with?
> 
> Can we only pref it off in Aurora or Beta?

Yes. I'm a little unclear on the merge mechanics though. Are we going to have to remember to pref it off each month post-merge?

> > ::: browser/themes/osx/devtools/font-inspector.css
> > @@ +3,5 @@
> > > +}
> > > +
> > > +body {
> > > +  background: #F9F9F9;
> > > +  font-family: Arial;
> > 
> > Arial. On OSX. In a font inspector. Isn't that some kind of heresy. Maybe go
> > the full monty and use Comic Sans? :)
> 
> I didn't know Arial was bad. What should I use then?
> 
> (later) ...
> I'll use the default font (I won't specify any).

This is a font panel. So I think it deserves to have some special thought go into fonts. I'm no font expert, so perhaps we should ask for Shorlander to take a quick look? Otherwise I don't have better suggestions than:
- Where Helvetica is available (i.e. OSX), it's probably a better option than Arial

Comment 31

[Joe Walker [:jwalker]]

Comment on attachment 720605 [details] [diff] [review]
patch v1.1

Review of attachment 720605 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/fontinspector/font-inspector.js
@@ +127,5 @@
> +    s = s.cloneNode(true);
> +
> +    s.querySelector(".font-name").textContent = font.name;
> +    s.querySelector(".font-css-name").textContent = font.CSSFamilyName;
> +    s.querySelector(".font-format").textContent = font.format;

I suspect that we could do this in far fewer lines of code using Template.jsm, but since we've got it working here, I don't feel a strong need to change it.

::: browser/devtools/fontinspector/font-inspector.xhtml
@@ +17,5 @@
> +  </head>
> +  <body class="devtools-monospace" role="application">
> +    <script type="application/javascript;version=1.8" src="font-inspector.js"></script>
> +    <script type="application/javascript;version=1.8">
> +    <![CDATA[

I was expecting to find somewhere that injected a script into here, but perhaps it's not needed?

@@ +26,5 @@
> +      <button id="showall" onclick="fontInspector.showAll()">&showAllFonts;</button>
> +    </div>
> +    <div id="template" style="display:none">
> +      <section class="font">
> +        <!-- FIXME: can we make sure this iframe doesn't have Chrome rights? -->

Yes, I think we can use the sandbox attribute:
https://developer.mozilla.org/en-US/docs/HTML/Element/iframe

Comment 32

[Paul Rouget [:paul]]

(In reply to Joe Walker [:jwalker] from comment #31)
> ::: browser/devtools/fontinspector/font-inspector.xhtml
> @@ +17,5 @@
> > +  </head>
> > +  <body class="devtools-monospace" role="application">
> > +    <script type="application/javascript;version=1.8" src="font-inspector.js"></script>
> > +    <script type="application/javascript;version=1.8">
> > +    <![CDATA[
> 
> I was expecting to find somewhere that injected a script into here, but
> perhaps it's not needed?

Forgot to remove it.

> @@ +26,5 @@
> > +      <button id="showall" onclick="fontInspector.showAll()">&showAllFonts;</button>
> > +    </div>
> > +    <div id="template" style="display:none">
> > +      <section class="font">
> > +        <!-- FIXME: can we make sure this iframe doesn't have Chrome rights? -->
> 
> Yes, I think we can use the sandbox attribute:
> https://developer.mozilla.org/en-US/docs/HTML/Element/iframe

Just tested. It works.

Comment 33

[Paul Rouget [:paul]]

Attachment: patch v1.2

Let's land that.

Comment 34

[Paul Rouget [:paul]]

Try: https://tbpl.mozilla.org/?tree=Try&rev=24ef05165033

Comment 35

[Paul Rouget [:paul]]

With new build system: https://tbpl.mozilla.org/?tree=Try&rev=f2b9c90e9e91

Comment 36

[Paul Rouget [:paul]]

Struggling with the new build system: https://tbpl.mozilla.org/?tree=Try&rev=249bf1e32647

Comment 37

[Paul Rouget [:paul]]

Attachment: patch v1.3 - rebased (with moz.build files)

Comment 38

[Paul Rouget [:paul]]

Attachment: patch v1.4 - fixes linux failure

Failure on Linux (no Arial font installed).

Re-re-re-re try: https://tbpl.mozilla.org/?tree=Try&rev=486d94d0b8e7

Comment 39

[Paul Rouget [:paul]]

Green enough to me.

Comment 40

[Paul Rouget [:paul]]

https://hg.mozilla.org/integration/fx-team/rev/2d7845ad3cd8

Comment 41

[Victor Porof [:vporof][:vp]]

https://hg.mozilla.org/mozilla-central/rev/2d7845ad3cd8

Comment 42

[Alex Keybl [:akeybl]]

This bug has been listed as part of the Aurora 22 release notes in either [1], [2], or both. If you find any of the information or wording incorrect in the notes, please email [e-mail] asap. Thanks!

[1] https://www.mozilla.org/en-US/firefox/22.0a2/auroranotes/
[2] https://www.mozilla.org/en-US/mobile/22.0a2/auroranotes/