The patch for Bug 1464796 accidentally removed a guard that now causes the FontEditor to show up in its empty state with the message "No fonts were found for the current element" even when the FontEditor pref is OFF. This has slipped into the 62 build, currently in Beta.
Comment on attachment 8988513 [details] Bug 1471914 - Check that the font editor pref is on before rendering it. https://reviewboard.mozilla.org/r/253780/#review260420 Looks good to me, and you confirmed that this worked fine (I didn't test it myself). Please let me know if you need any help getting it uplifted since this will be your first beta uplift request.
Attachment #8988513 - Flags: review?(pbrosset) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/988ea9310c36 Check that the font editor pref is on before rendering it. r=pbro
Comment on attachment 8988513 [details] Bug 1471914 - Check that the font editor pref is on before rendering it. Approval Request Comment [Feature/Bug causing the regression]: Patch for Bug 1464796 accidentally removed a condition which makes the default empty state of the Font editor (which is in development) visible to users who don't have the pref turned on to see it. [User impact if declined]: Users see a confusing message in the Fonts panel that "No fonts were found for this element" right above the list of fonts which were found. [Is this code covered by automated tests?]: Not completely. The existing Font panel has tests. The upcoming Font Editor tests are being written now. [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: Yes. Steps: 1. Open a new tab page 2. Open DevTools and select the "<body>" tag in the Elements panel 3. Switch to "Fonts" sidebar panel Expected: The message "No fonts were found for the current element" should not appear in the Fonts panel. A list of one or more fonts should show up. [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: Unlikely. It restores UI as it was prior to this bug occurring. [Why is the change risky/not risky?]: It reintroduces a condition that was previously available. Instead of its boolean value coming from checking an array's length, it now comes from checking the pref value for enabling the font editor: devtools.inspector.fonteditor.enabled [String changes made/needed]: None
Attachment #8988513 - Flags: approval-mozilla-beta?
Comment on attachment 8988513 [details] Bug 1471914 - Check that the font editor pref is on before rendering it. Put back a needed pref check, please uplift for beta 7.
Attachment #8988513 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I successfully reproduced the issue on Nightly 63.0a1 (2018-06-28) under Windows 10 (x64) using STR from Comment 5. The issue is no longer reproducible on Firefox 62.0, Firefox Beta 63.0b4 and latest Nightly 64.0a1 (2018-09-10) under Windows 10 (x64), Ubuntu 18.04 (x64) and macOS 10.13.
You need to log in before you can comment on or make changes to this bug.