Closed Bug 1464336 Opened 2 years ago Closed 2 years ago

font inspector confuses font family name vs full font (face) name

Categories

(DevTools :: Inspector: Fonts, defect, P3)

defect

Tracking

(firefox62 fixed)

RESOLVED FIXED
Firefox 62
Tracking Status
firefox62 --- fixed

People

(Reporter: jfkthame, Assigned: rcaliman)

References

Details

Attachments

(1 file)

The inspector is a little confused about naming, when showing the font(s) used by the content being inspected.

The first thing it shows is labelled "Family", but what it actually presents there is the name of the specific _face_, which is not (in general) the same as the _family_ name and may well not be usable as a value for CSS font-family.

Trivial example: on macOS, with the testcase

  data:text/html,<div style="font-family:Times">Hello <b>world</b>

open the Inspector on the <b> element. It shows "Family: Times Bold". But the correct _family_ name would be "Times"; the name "Times Bold" should be referred to as the "font face" (or just "font"), to avoid implying that it corresponds to CSS font-family property.

Inspect the <div> element, and it's even more confusing: two fonts are listed, named as "Family: Times Roman" and "Family: Times Bold". Neither of these are the _family_ name, which is still "Times".
Assignee: nobody → rcaliman
Blocks: 1441576
Status: NEW → ASSIGNED
Priority: -- → P3
Comment on attachment 8981121 [details]
Bug 1464336 - Font Editor: Show font family AND font name in editor and overview.

https://reviewboard.mozilla.org/r/247212/#review253294
Attachment #8981121 - Flags: review?(pbrosset) → review+
Pushed by rcaliman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/684e4b87653e
Font Editor: Replace font name with CSSFamilyName in editor and overview. r=pbro
Backed out changeset 684e4b87653e (bug 1464336) for Devtools on devtools/client/inspector/fonts/test/browser_fontinspector.js. CLOSED TREE 

Log:
https://treeherder.mozilla.org/logviewer.html#?job_id=180575600&repo=autoland&lineNumber=1592

TEST-PASS | devtools/client/inspector/fonts/test/browser_fontinspector.js | Found 5 fonts - 
07:44:57     INFO - Buffered messages finished
07:44:57     INFO - TEST-UNEXPECTED-FAIL | devtools/client/inspector/fonts/test/browser_fontinspector.js | font 0 right font name - Got bar, expected Ostrich Sans Medium
07:44:57     INFO - Stack trace:
07:44:57     INFO - chrome://mochikit/content/browser-test.js:test_is:1285
07:44:57     INFO - chrome://mochitests/content/browser/devtools/client/inspector/fonts/test/browser_fontinspector.js:testBodyFonts:53
07:44:57     INFO - chrome://mochitests/content/browser/devtools/client/inspector/fonts/test/browser_fontinspector.js:null:37
07:44:57     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1083
07:44:57     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1074
07:44:57     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:976
07:44:57     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:795
07:44:57     INFO - TEST-PASS | devtools/client/inspector/fonts/test/browser_fontinspector.js | font 0 remote value correct - 
07:44:57     INFO - TEST-PASS | devtools/client/inspector/fonts/test/browser_fontinspector.js | font 0 url correct - 
07:44:57     INFO - Not taking screenshot here: see the one that was previously logged
07:44:57     INFO - TEST-UNEXPECTED-FAIL | devtools/client/inspector/fonts/test/browser_fontinspector.js | font 1 right font name - Got bar, expected Ostrich Sans Black
07:44:57     INFO - Stack trace:
07:44:57     INFO - chrome://mochikit/content/browser-test.js:test_is:1285
07:44:57     INFO - chrome://mochitests/content/browser/devtools/client/inspector/fonts/test/browser_fontinspector.js:testBodyFonts:53
07:44:57     INFO - chrome://mochitests/content/browser/devtools/client/inspector/fonts/test/browser_fontinspector.js:null:37
07:44:57     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1083
07:44:57     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1074
07:44:57     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:976
07:44:57     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:795
07:44:57     INFO - TEST-PASS | devtools/client/inspector/fonts/test/browser_fontinspector.js | font 1 remote value correct - 

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=684e4b87653e5a1bc4c760e257ea80a76f82af7f&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified

Backout:
https://hg.mozilla.org/integration/autoland/rev/f3413f38e54a9d40981a5ce231bc4ce048d97798
Flags: needinfo?(rcaliman)
New patch to revert changes that failed the test. The font family name is now added alongside the font name instead of replacing it.
Flags: needinfo?(rcaliman)
Comment on attachment 8981121 [details]
Bug 1464336 - Font Editor: Show font family AND font name in editor and overview.

https://reviewboard.mozilla.org/r/247212/#review253744

Looks great. I agree with showing the 2 names.
Pushed by rcaliman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/53a3726088f5
Font Editor: Show font family AND font name in editor and overview. r=pbro
Backout by rgurzau@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/780e7e87327d
Backed out changeset 53a3726088f5 for failures on /inspector/fonts/test/browser_fontinspector_reveal-in-page.js on a CLOSED TREE
Backed out changeset 53a3726088f5 (bug 1464336) for failures on /inspector/fonts/test/browser_fontinspector_reveal-in-page.js on a CLOSED TREE 

Backout link: https://hg.mozilla.org/integration/autoland/rev/780e7e87327d4a575b2b29a7e438660b0c14f5d3

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=53a3726088f55287efcd48767dd40e1b20404bb3

Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=180863013&repo=autoland&lineNumber=2207

Log snippet: 
10:31:50     INFO -  479 INFO TEST-START | devtools/client/inspector/fonts/test/browser_fontinspector_reveal-in-page.js
10:31:50     INFO -  GECKO(3288) | console.log: "[DISPATCH]" "{\n  \"type\": \"UPDATE_FONTS\",\n  \"fonts\": [],\n  \"otherFonts\": []\n}"
10:31:50     INFO -  GECKO(3288) | console.log: "[DISPATCH]" "{\n  \"type\": \"UPDATE_FONTS\",\n  \"fonts\": [],\n  \"otherFonts\": []\n}"
10:31:50     INFO -  GECKO(3288) | console.log: "[DISPATCH]"
10:31:50     INFO -  GECKO(3288) | console.log: "[DISPATCH]"
10:32:35     INFO -  TEST-INFO | started process screenshot
10:32:35     INFO -  TEST-INFO | screenshot: exit 0
10:32:35     INFO -  Buffered messages logged at 10:31:50
10:32:35     INFO -  480 INFO Entering test bound
10:32:35     INFO -  481 INFO Adding a new tab with URL: http://example.com/browser/devtools/client/inspector/fonts/test/browser_fontinspector.html
10:32:35     INFO -  482 INFO Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "http://example.com/browser/devtools/client/inspector/fonts/test/browser_fontinspector.html" line: 0}]
10:32:35     INFO -  483 INFO Tab added and finished loading
10:32:35     INFO -  484 INFO Opening the inspector
10:32:35     INFO -  485 INFO Opening the toolbox
10:32:35     INFO -  486 INFO Console message: [JavaScript Error: "downloadable font: download failed (font-family: "bar" style:normal weight:400 stretch:100 src index:0): status=2147746065 source: http://example.com/browser/devtools/client/inspector/fonts/test/bad/font/name.ttf" {file: "unknown" line: 4 column: 13 source: "@font-face {
10:32:35     INFO -    font-family: bar;
10:32:35     INFO -    src: url("bad/font/name.ttf"), url("ostrich-regular.ttf") format("truetype");
10:32:35     INFO -  }"}]
10:32:35     INFO -  487 INFO Toolbox opened and focused
10:32:35     INFO -  488 INFO Selecting the node for 'body'
10:32:35     INFO -  489 INFO Mousing over and out of font number 0 in the list
10:32:35     INFO -  490 INFO TEST-PASS | devtools/client/inspector/fonts/test/browser_fontinspector_reveal-in-page.js | 1 selectionchange events detected on mouseover -
10:32:35     INFO -  491 INFO TEST-PASS | devtools/client/inspector/fonts/test/browser_fontinspector_reveal-in-page.js | 1 selectionchange events detected on mouseout -
10:32:35     INFO -  492 INFO Mousing over and out of font number 1 in the list
10:32:35     INFO -  493 INFO TEST-PASS | devtools/client/inspector/fonts/test/browser_fontinspector_reveal-in-page.js | 2 selectionchange events detected on mouseover -
10:32:35     INFO -  494 INFO TEST-PASS | devtools/client/inspector/fonts/test/browser_fontinspector_reveal-in-page.js | 1 selectionchange events detected on mouseout -
10:32:35     INFO -  495 INFO Mousing over and out of font number 2 in the list
10:32:35     INFO -  496 INFO TEST-PASS | devtools/client/inspector/fonts/test/browser_fontinspector_reveal-in-page.js | 2 selectionchange events detected on mouseover -
10:32:35     INFO -  497 INFO TEST-PASS | devtools/client/inspector/fonts/test/browser_fontinspector_reveal-in-page.js | 1 selectionchange events detected on mouseout -
10:32:35     INFO -  498 INFO Mousing over and out of font number 3 in the list
10:32:35     INFO -  499 INFO TEST-PASS | devtools/client/inspector/fonts/test/browser_fontinspector_reveal-in-page.js | 1 selectionchange events detected on mouseover -
10:32:35     INFO -  500 INFO TEST-PASS | devtools/client/inspector/fonts/test/browser_fontinspector_reveal-in-page.js | 1 selectionchange events detected on mouseout -
10:32:35     INFO -  501 INFO Mousing over and out of font number 4 in the list
10:32:35     INFO -  Buffered messages finished
10:32:35    ERROR -  502 INFO TEST-UNEXPECTED-FAIL | devtools/client/inspector/fonts/test/browser_fontinspector_reveal-in-page.js | Test timed out -
Flags: needinfo?(rcaliman)
The patch provided is now obsolete. The fix for this now depends on changes in Bug 1464796. Once that goes ahead, I will provide an updated patch and corresponding tests.
Depends on: 1464796
Flags: needinfo?(rcaliman)
Product: Firefox → DevTools
Pushed by rcaliman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7110e2638671
Font Editor: Show font family AND font name in editor and overview. r=pbro
https://hg.mozilla.org/mozilla-central/rev/7110e2638671
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
You need to log in before you can comment on or make changes to this bug.