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

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P3)

Description

[Jonathan Kew [:jfkthame]]

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".

Comment 1

[Razvan Caliman [:rcaliman]]

Attachment: Bug 1464336 - Font Editor: Show font family AND font name in editor and overview.

Review commit: https://reviewboard.mozilla.org/r/247212/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/247212/

Comment 2

[Patrick Brosset <:pbro>]

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

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/684e4b87653e
Font Editor: Replace font name with CSSFamilyName in editor and overview. r=pbro

Comment 4

[Dorel Luca [:dluca]]

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

Comment 5

[Razvan Caliman [:rcaliman]]

Comment on attachment 8981121 [details]
Bug 1464336 - Font Editor: Show font family AND font name in editor and overview.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/247212/diff/1-2/

Comment 6

[Razvan Caliman [: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.

Comment 7

[Patrick Brosset <:pbro>]

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.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/53a3726088f5
Font Editor: Show font family AND font name in editor and overview. r=pbro

Comment 9

[Pulsebot]

Backout by [e-mail]:
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

Comment 10

[Raul Gurzau (:RaulG)]

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 -

Comment 11

[Razvan Caliman [: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.

Comment 12

[Razvan Caliman [:rcaliman]]

Comment on attachment 8981121 [details]
Bug 1464336 - Font Editor: Show font family AND font name in editor and overview.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/247212/diff/2-3/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7110e2638671
Font Editor: Show font family AND font name in editor and overview. r=pbro

Comment 14

[Razvan Caliman [:rcaliman]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a7a481a8289e7152dea4e952cec69669d2a1741f

Comment 15

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/7110e2638671