1247723 - Disable the Font Panel

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Helen V. Holmes (:helenvholmes) (:✨)]

The font panel is not currently very useful. According to Patrick/telemetry, the average user spends ~5 seconds it in, and it seems like even that might be accidental: https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2016-01-25&keys=__none__!__none__!__none__&max_channel_version=aurora%252F45&measure=DEVTOOLS_FONTINSPECTOR_TIME_ACTIVE_SECONDS&min_channel_version=null&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2015-12-17&table=0&trim=1&use_submission_date=0

I'd like to eventually create a font panel that's robust and useful (exposes Opentype features like ligatures and smallcaps, allows you to find glyphs in a font file, things of that nature). I'm proposing until we have the bandwidth to work on something of that nature we disable the font panel, as it creates additional, unuseful noise in what are fairly crowded devtools as it is.

According to Patrick this should be fairly easy and involves setting devtools.fontinspector.enabled to false.

Comment 1

[mark.kazakevich]

~University of Toronto, CSC302H1S-2016 assignment. Patch by Feb 26,2016~

I would like to work on this as my first FF bug fix.
I'm running on Linux Mint. I have checked out the gecko-dev source code, built FF 46 and am able to run it.

1) Would somebody advise me where to look for the necessary source code?
2) Any tips/suggestions? 

Thank you.

Comment 2

[Helen V. Holmes (:helenvholmes) (:✨)]

(In reply to mark.kazakevich from comment #1)
> ~University of Toronto, CSC302H1S-2016 assignment. Patch by Feb 26,2016~
> 
> I would like to work on this as my first FF bug fix.
> I'm running on Linux Mint. I have checked out the gecko-dev source code,
> built FF 46 and am able to run it.
> 
> 1) Would somebody advise me where to look for the necessary source code?
> 2) Any tips/suggestions? 
> 
> Thank you.

Hey Mark! Thanks for volunteering!

Patrick Brosset is the mentor for this bug. On IRC, he's :pbro—there are some instructions on getting on IRC here: https://wiki.mozilla.org/IRC 

The channel you can ask questions in is #devtools.

The code for devtools is at the top level, /devtools/. You'll have to grep for the right flag.

I'm going to assign the bug to you on Bugzilla.

Comment 3

[Vishnu (:malayaleecoder)]

@mark: Please let me know if you are still going for the fix, else I am ready to take this up!

Comment 4

[Vishnu (:malayaleecoder)]

@Patrick: looks like mark is not active for almost a week. Can I have a go at this,if you don't mind ?

Comment 5

[mark.kazakevich]

Hey, sorry I've have been on break this week.
However, you may take it. I'm likely going with something else now.
Cheers.

Comment 6

[Vishnu (:malayaleecoder)]

Oh, Thanks Mark!

Patrick, should I change the boolean value for 'devtools.fontinspector.enabled' present in,
obj-x86_64-unknown-linux-gnu/dist/bin/browser/defaults/preferences/devtools.js 
also?

Comment 7

[Patrick Brosset <:pbro>]

(In reply to malayaleecoder from comment #6)
> Patrick, should I change the boolean value for
> 'devtools.fontinspector.enabled' present in,
> obj-x86_64-unknown-linux-gnu/dist/bin/browser/defaults/preferences/devtools.
> js 
> also?
No, the directory that starts with obj- is your build directory, it's created automatically by mach when you build firefox, and it contains the resulting binaries. You never need to change anything in that directory.

Comment 8

[Vishnu (:malayaleecoder)]

Attachment: Bug1247723_v1.diff

Hello Patrick,

I have done the changes as said in comment #1 in this patch.
But on running ./mach run, I still see the "Font & Colors" panel.
Is there something wrong that I have done, or am I addressing the wrong panel?

Comment 9

[Brian Grinstead [:bgrins]]

(In reply to malayaleecoder from comment #8)
> Created attachment 8720814 [details] [diff] [review]
> Bug1247723_v1.diff
> 
> Hello Patrick,
> 
> I have done the changes as said in comment #1 in this patch.
> But on running ./mach run, I still see the "Font & Colors" panel.
> Is there something wrong that I have done, or am I addressing the wrong
> panel?

Try running `./mach build faster` after making the changes but before doing `./mach run`

Comment 10

[Vishnu (:malayaleecoder)]

(In reply to Brian Grinstead [:bgrins] from comment #9)
> (In reply to malayaleecoder from comment #8)
> > Created attachment 8720814 [details] [diff] [review]
> > Bug1247723_v1.diff
> > 
> > Hello Patrick,
> > 
> > I have done the changes as said in comment #1 in this patch.
> > But on running ./mach run, I still see the "Font & Colors" panel.
> > Is there something wrong that I have done, or am I addressing the wrong
> > panel?
> 
> Try running `./mach build faster` after making the changes but before doing
> `./mach run`

Brian, I did do the './mach build faster' before './mach run', but am still unable to find any changes!
Any help?

Comment 11

[Brian Grinstead [:bgrins]]

You mention the "Font & Colors" panel - I'm not sure what that is.  This is a sidebar panel inside the Inspector tool. See "Fonts View" here: https://developer.mozilla.org/en-US/docs/Tools/Page_Inspector/How_to/View_fonts

Comment 12

[Patrick Brosset <:pbro>]

I think that's because you have the pref defined in about:config. The fix for this bug is to just turn the default value of that pref to false. But for everyone that already has that pref set, the set value will still be taken into account.
Open about:config, find the pref, and delete it from there. Then re-start devtools

Comment 13

[Vishnu (:malayaleecoder)]

Thank You Brian, I was actually looking at the wrong panel!

@Patrick, I think the bug is fixed, I can no longer see the Font Panel on running. Please verify and tell me if further improvements are required.

Sorry for the confusion :\

Comment 14

[Patrick Brosset <:pbro>]

Comment on attachment 8720814 [details] [diff] [review]
Bug1247723_v1.diff

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

Looks good.
Now, we need to make sure the tests do work still. There are a bunch of tests for the font panel. I was going to suggest that we disable them, but the thing is, what we're doing here is simply turn off the pref, which means people can still turn it on. And for them, we want the panel to still work. We therefore want the tests to still run, especially to catch possible regressions in the future.
Tests are in \devtools\client\inspector\fonts\test\
So, to make sure the tests still work, you'll need to add something like this to the head.js file in this directory (this file is run before each and every test):

Services.prefs.setBoolPref("devtools.fontinspector.enabled", true);
registerCleanupFunction(() => {
  Services.prefs.clearUserPref("devtools.fontinspector.enabled");
});

This will turn the pref ON before the tests start, and will turn it back OFF when the tests end.
Once done, please do run the tests locally to make sure they work: ./mach test devtools/client/inspector/fonts/test/

Also, can you please change the commit message to be:
"Bug 1247723 - Disable the Font Panel; r=pbro"

Comment 15

[Vishnu (:malayaleecoder)]

Attachment: Bug1247723_v2.diff

Hey Patrick,

I have made the suggested changes and all the tests ran successfully!
Please have a look.

Thank You

Comment 16

[Patrick Brosset <:pbro>]

Comment on attachment 8721429 [details] [diff] [review]
Bug1247723_v2.diff

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

Looks good to me.
I have pushed this patch to CI to make sure all tests still pass: https://treeherder.mozilla.org/#/jobs?repo=try&revision=00e8e0bf8246

Comment 17

[Patrick Brosset <:pbro>]

Alright, the TRY push is green, and the patch is R+, this is ready to land on fx-team.
Bye bye fonts panel, see you soon!

Comment 18

[Vishnu (:malayaleecoder)]

Thanks Patrick for helping me out :-)

Comment 19

[Lina Butler [:lina]]

https://hg.mozilla.org/integration/fx-team/rev/b3175dd140e73e6d4875a99d4299a15539571f01
Bug 1247723 - Disable the Font Panel; r=pbro

Comment 20

[Lina Butler [:lina]]

https://hg.mozilla.org/mozilla-central/rev/b3175dd140e7

Comment 21

[Will Bamberg [:wbamberg]]

doc updates:
https://developer.mozilla.org/en-US/docs/Tools/Page_Inspector/How_to/View_fonts#Fonts_view
https://developer.mozilla.org/en-US/docs/Tools/Page_Inspector/UI_Tour#Fonts_view

Comment 22

[Gavin]

just wanted to give a bit of feedback as a user of the font panel, ~5 seconds is actually plenty for what it currently offers so that metric alone may not be the best data to use for deciding to disable it by default.

I for example, use it to ensure the correct font has loaded when using multiple fonts in a family. Using the CSS inspector alone isnt enough as it doesnt tell you which fonts are in use, just what was declared. A quick toggle of the font tab to dbl check is all that's needed.

The font panel is one my chrome using colleagues are a little envious of.

Comment 23

[Brian Grinstead [:bgrins]]

(In reply to Gavin from comment #22)
> just wanted to give a bit of feedback as a user of the font panel, ~5
> seconds is actually plenty for what it currently offers so that metric alone
> may not be the best data to use for deciding to disable it by default.
> 
> I for example, use it to ensure the correct font has loaded when using
> multiple fonts in a family. Using the CSS inspector alone isnt enough as it
> doesnt tell you which fonts are in use, just what was declared. A quick
> toggle of the font tab to dbl check is all that's needed.

Good point - that's use case that the rule view doesn't cover.  We have Bug 994559 on file which is about enhancing the rule view / computed view to somehow highlight only the applied font.  In fact, that would be more convenient than needing to switch to another panel.

Comment 24

[Jeff Muizelaar [:jrmuizel]]

It seems like disabling this is a little user hostile. I'm not sure what we gain from breaking existing users before we have a working replacement for exposing this information.

See this reddit thread for more:
https://www.reddit.com/r/firefox/comments/4itktq/starting_in_firefox_47_the_fonts_view_is_disabled/

Comment 25

[flying sheep]

Why would you remove it without a replacement ready?

This is just useful information being gone.

Please revert this.

Comment 26

[yisibl]

Update https://developer.mozilla.org/en-US/Firefox/Releases/47#Developer_Tools

Comment 27

[yisibl]

Regardless of the developer or designer, the font panel is very useful. I even suggested that Chrome add a separate font panel. I would like to be able to re design the layout of the panel instead of the default disable it.

Comment 28

[yisibl]

Attachment: Chrome-DevTools.png

Comment 29

[yisibl]

Comment on attachment 8761027 [details]
Chrome-DevTools.png

Chrome Devtools will automatically fold the panel.

Comment 30

[fireattack [:fireattack]]

Isn't font inspection tool *essential* for web developers, especially frond-end developers? Disabling it by default (without a replacement) will just push people to use Chrome to do web dev further more in my humble opinion.

And I didn't get the logic that "it's not currently very useful because people rarely use it". Usefulness and usage frequency / popularity are two different things: to make a rough analogy, maybe only 0.001% people on the Earth ever use a Geiger counter but no one will claim it's "not very useful". 

It's funny because you said you are going to add more features in the future such as "exposes Opentype features like ligatures and smallcaps, allows you to find glyphs in a font file, things of that nature": because these features will not increase a single bit of the popularity of fool inspector. Front-end dev will use it all the time, no matter if it sucks or not; while average Joe may never know it exists.

It's totally irrelevant to the features of the font tool. If you want to make it more "useful" and robust, Sure, why not? But if your reason to disable it is because "people don't use it enough", adding more features will not help this goal at all. Hence it's no need to disable it before the replacement is ready to ship.

Comment 31

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

It is still possible to re-enable the font panel for your local use if you wish, it has only been hidden for now.

You can go to about:config and change devtools.fontinspector.enabled to true if you'd like to unhide it.

Comment 32

[Terrell Kelley]

Yes, it's been disabled. But you didn't put a preference in the actual DevTools settings, so people won't know that they can enable it.

Comment 33

[Julian Descottes [:jdescottes]]

Logged bug 1278984 in order to make the Fonts panel available from the sidebar menu.

Comment 34

[Florian Rivoal]

If I understand correctly, this is supposed to be replaced by https://bugzilla.mozilla.org/show_bug.cgi?id=994559. However, that seems short-sighted, since at least the initial description of that bug makes incorrect assumptions about how fonts work in CSS. It assumes that only one font in the list of fallback will be applied, which isn't true. At also glosses over the fact that one font family can be composed of several font faces, and white the Font View does give details about it, that suggestion wouldn't

I have nothing against replacing the Font View with some actually improved replacement, but retiring it in favor of a vague idea that has not yet been thought through let alone implemented, seems premature.

And yes, I know this is merely hidden, not removed; but this still effectively means the feature no longer exists for most people, and this is/was one of the areas where Firefox's dev tools are superior to the competition.

Comment 35

[Sebastian Zartner [:sebo]]

(In reply to Florian Rivoal from comment #34)
> If I understand correctly, this is supposed to be replaced by
> https://bugzilla.mozilla.org/show_bug.cgi?id=994559.

No, comment 0 says that the panel will be reworked at some point. The bug you link to covers only one feature of the Font Inspector. Having said that, I didn't see a bug filed for the new Font Inspector. Helen, is there already one?

> However, that seems
> short-sighted, since at least the initial description of that bug makes
> incorrect assumptions about how fonts work in CSS. It assumes that only one
> font in the list of fallback will be applied, which isn't true.

I don't see the initial description making such assumptions. It just says that the reworked Font Inspector should provide more features.

> I have nothing against replacing the Font View with some actually improved
> replacement, but retiring it in favor of a vague idea that has not yet been
> thought through let alone implemented, seems premature.
> 
> And yes, I know this is merely hidden, not removed; but this still
> effectively means the feature no longer exists for most people, and this
> is/was one of the areas where Firefox's dev tools are superior to the
> competition.

I absolutely agree on that.
The telemetry data doesn't answer important questions like how people use the tool or by how many people it is used intentionally. As Gavin mentioned in comment 22 five seconds may be enough for people to get from the panel what they want, so accidental switches to it can't be extracted from the telemetry data.

Sebastian

Comment 36

[Sebastian Zartner [:sebo]]

Btw. adding the panel back (or fixing bug 1278984) is not for the users carefully reading this bug or the docs and by that knowing that they can reenable the feature. It is for those users that suddenly see the Fonts panel gone for no obvious reason and not finding the option to get it back.

Sebastian

Comment 37

[Florian Rivoal]

>> However, that seems
>> short-sighted, since at least the initial description of that bug makes
>> incorrect assumptions about how fonts work in CSS. It assumes that only one
>> font in the list of fallback will be applied, which isn't true.
>
> I don't see the initial description making such assumptions. It just says that the reworked Font Inspector should provide more features.

By "that bug", I meant Bug 994559, which was the only one I could find listed from here as an alternative to this.

Comment 38

[Helen V. Holmes (:helenvholmes) (:✨)]

Clearing my ni? based on comment #33, which will allow the Font panel to be re-enabled.

Comment 39

[Sebastian Zartner [:sebo]]

(In reply to Helen V. Holmes (:helenvholmes) (:✨)(pls ni?)(limited avail 14-17) from comment #38)
> Clearing my ni? based on comment #33, which will allow the Font panel to be
> re-enabled.

I was asking whether there is already a bug filed for the *new* Font Inspector you mentioned in comment 0, not about reenabling the existing one.

Sebastian

Comment 40

[Helen V. Holmes (:helenvholmes) (:✨)]

No, no bug has been filed yet.

Comment 41

[Sebastian Zartner [:sebo]]

(In reply to Helen V. Holmes (:helenvholmes) (:✨)(pls ni?)(limited avail 14-17) from comment #40)
> No, no bug has been filed yet.

Ok, so I've created bug 1280059 to track it. Feel free to add bugs blocking it.

Sebastian

Comment 42

[Paul Rouget [:paul]]

I'm surprised this happened.

Don't we want to fix the before-mentioned issues before removing this tool?

Also, I don't see how bug 994559 would help. The features offered by the font panel are not directly related to the rule view or css.

And what is bug 1280059 supposed to track?

Comment 43

[Sebastian Zartner [:sebo]]

(In reply to Paul Rouget [:paul] from comment #42)
> And what is bug 1280059 supposed to track?

The features Helen mentioned in comment 0 that are making it "robust and useful".

Sebastian

Comment 44

[Paul Rouget [:paul]]

(In reply to Sebastian Zartner [:sebo] from comment #43)
> (In reply to Paul Rouget [:paul] from comment #42)
> > And what is bug 1280059 supposed to track?
> 
> The features Helen mentioned in comment 0 that are making it "robust and
> useful".

Please expand. Lack of opentype features can't justify killing this tool, especially when it worked well.

As the author of this tool, I deserve some propers explanations on why this got killed without any proper replacements or a clear description of requirements for a v2.

Comment 45

[Paul Rouget [:paul]]

Patrick, can you please comment on this?

If we want to create a v2, why do we kill the v1 before the v2 is ready? What's the benefit?

Comment 46

[Sebastian Zartner [:sebo]]

(In reply to Paul Rouget [:paul] from comment #44)
> (In reply to Sebastian Zartner [:sebo] from comment #43)
> > (In reply to Paul Rouget [:paul] from comment #42)
> > > And what is bug 1280059 supposed to track?
> > 
> > The features Helen mentioned in comment 0 that are making it "robust and
> > useful".
> 
> Please expand. Lack of opentype features can't justify killing this tool,
> especially when it worked well.

Forwarding this request to Helen.
I agree with you that the current Fonts panel should be re-enabled independently of the work on new features. Therefore I have now created bug 1280121.

Sebastian

Comment 47

[Patrick Brosset <:pbro>]

One problem we often face with devtools is getting feedback. We can never know how, how much, and for what people use our tools. And so we try and take decisions as best we can to provide the best tools we can, in our opinions, given the feedback and data we have at our disposal.

The original intent behind hiding the fonts panel by default was that that it created additional noise in a fairly complex UI and, if you compared it to other panels in the inspector, didn't seem like the most important tool to have.

We want a less intimidating UI that is less crowded, more accessible to everyone with the tools people really need easily available. Again, with the data and feedback we had at our disposal, the fonts panel didn't appear to fit in that picture very well.

Also, one of the most important things we can do as team working on devtools is focusing our energy on less but better things. We used to have a very rapid expansion phase at the start where we didn't focus on anything but instead did millions of different things which, later, went unmaintained.

A consequence of wanting to focus more is that we might need to remove (or disable in this case) things we know we don't have the time to work on right now and properly finish.

So, we made a decision and went with it. And we learned here that people relied on that functionality more than we thought. I think people made really good points here.
So, thank you all for the feedback, and thank you Sebastian for taking care of filing and linking bugs.

I think we should stop the conversation here and direct our energy to the other bugs mentioned here, whether that means introducing a checkbox in the devtools settings panel, making the sidebar tabs widget look better/cleaner and still hold all our panels, working on a v2 now, or just straight on reverting the change.

Comment 48

[u525076]

Just updated to 47 and got surprised in 1 minute :D, came here by Googling "firefox inspector font removed".

In my opinion, at least for developers, cluttered UI is less worse than missing/hidden features. The font panel was very useful to me.

Comment 49

[Sebastian Zartner [:sebo]]

Bagana, please read comment 47, especially the last paragraph.
So, I advise you to follow the bugs listed under 'Blocks' and 'See Also', which are bug 1280121, bug 994559, bug 1278984 and bug 1280059.

Sebastian