Closed Bug 1471914 Opened 7 years ago Closed 7 years ago

Font editor: Empty state for FontEditor mistakenly shows up when its pref is OFF

Categories

(DevTools :: Inspector, defect, P1)

Product:
DevTools
Component:
Inspector
Type:
defect

Tracking

(firefox-esr52 unaffected, firefox-esr60 unaffected, firefox61 unaffected, firefox62 verified, firefox63 verified, firefox64 verified)

Status:
VERIFIED FIXED
Milestone:
Firefox 63
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 --- verified
firefox63 --- verified
firefox64 --- verified

People

(Reporter: rcaliman, Assigned: rcaliman)

References

Details

(Keywords: regression)

Attachments

(2 files)

Empty state for font editor should not show up.
560.13 KB, image/jpeg
Details
Bug 1471914 - Check that the font editor pref is on before rendering it.
59 bytes, text/x-review-board-request
pbro
: review+
lizzard
: approval-mozilla-beta+
Details
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 rcaliman@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/988ea9310c36 Check that the font editor pref is on before rendering it. r=pbro
https://hg.mozilla.org/mozilla-central/rev/988ea9310c36
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox63: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
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+
Flags: qe-verify+
QA Contact: catalin.sasca
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.
Status: RESOLVED → VERIFIED
status-firefox62: fixed → verified
status-firefox63: fixed → verified
status-firefox64: --- → verified
Flags: qe-verify+
Component: Inspector: Fonts → Inspector
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: