Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the font Linux Libertine as a possible substitution for Times New Roman #17405

Merged
merged 1 commit into from
Jan 16, 2024

Conversation

calixteman
Copy link
Contributor

@calixteman calixteman commented Dec 10, 2023

and try to load the font family (guessed from the font name) before trying
the local substitution.
The local(...) command expects to have a real font name and not a predefined
substitution it's why we try the font family.

@Snuffleupagus
Copy link
Collaborator

I'm guessing that this patch is related to issue #17401? If so, can you please get the user to confirm that it fixes the problem?

/botio-linux preview

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/70ae51edc48ebfe/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/70ae51edc48ebfe/output.txt

Total script time: 1.46 mins

Published

@jwillia3
Copy link

On both Viewer and Legacy Viewer, Nimbus Roman is selected. (See screenshot)

Nimbus Roman is still being selected and displayed even when fontconfig excludes the font and even when the files are deleted from the system.

It is now selecting Nimbus Roman over DejaVu Serif, but I can't find a way to get Linux Libertine O to be selected. Also note, people may have Linux Libertine installed and not Linux Libertine O.

Re: DejaVu Serif, would it also not be good to remove that from the list since it is not metric compatible?

Nimbus-Roman

@calixteman
Copy link
Contributor Author

/botio-linux preview

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_preview from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/91c07af1ae6be73/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/91c07af1ae6be73/output.txt

Total script time: 1.42 mins

Published

@calixteman
Copy link
Contributor Author

@jwillia3 could you try again with the viewer in #17405 (comment) ?

@jwillia3
Copy link

It seems like it's using Nimbus Roman no matter what fonts I have installed.
Screenshot 2024-01-15 at 02-56-44 compressed tracemonkey-pldi-09 pdf

It does not select Linux Libertine when available even when Nimbus Roman is not installed.

I tried removed all fonts (including Nimbus Roman) except for the list below and it still selected Nimbus Roman.

luximr.ttf: "Luxi Mono" "Regular"
luxirr.ttf: "Luxi Serif" "Regular"
luxisr.ttf: "Luxi Sans" "Regular"
c0419bt_.pfb: "Courier 10 Pitch" "Regular"
UTRG____.pfa: "Utopia" "Regular"
l048013t.pfa: "Luxi Sans" "Regular"
c0648bt_.pfb: "Bitstream Charter" "Regular"
l049013t.pfa: "Luxi Serif" "Regular"
l047013t.pfa: "Luxi Mono" "Regular"
cour.pfa: "Courier" "Regular"
luxirb.ttf: "Luxi Serif" "Bold"
luxisb.ttf: "Luxi Sans" "Bold"
luximb.ttf: "Luxi Mono" "Bold"
courb.pfa: "Courier" "Bold"
UTB_____.pfa: "Utopia" "Bold"
c0583bt_.pfb: "Courier 10 Pitch" "Bold"
c0632bt_.pfb: "Bitstream Charter" "Bold"
l047016t.pfa: "Luxi Mono" "Bold"
l048016t.pfa: "Luxi Sans" "Bold"
l049016t.pfa: "Luxi Serif" "Bold"
UTI_____.pfa: "Utopia" "Italic"
c0582bt_.pfb: "Courier 10 Pitch" "Italic"
c0649bt_.pfb: "Bitstream Charter" "Italic"
couri.pfa: "Courier" "Italic"
courbi.pfa: "Courier" "Bold Italic"
UTBI____.pfa: "Utopia" "Bold Italic"
c0611bt_.pfb: "Courier 10 Pitch" "Bold Italic"
c0633bt_.pfb: "Bitstream Charter" "Bold Italic"
luxirri.ttf: "Luxi Serif" "Oblique"
luxisri.ttf: "Luxi Sans" "Oblique"
luximri.ttf: "Luxi Mono" "Oblique"
l047033t.pfa: "Luxi Mono" "Oblique"
l049033t.pfa: "Luxi Serif" "Oblique"
l048033t.pfa: "Luxi Sans" "Oblique"
luximbi.ttf: "Luxi Mono" "Bold Oblique"
luxirbi.ttf: "Luxi Serif" "Bold Oblique"
luxisbi.ttf: "Luxi Sans" "Bold Oblique"
l049036t.pfa: "Luxi Serif" "Bold Oblique"
l047036t.pfa: "Luxi Mono" "Bold Oblique"
l048036t.pfa: "Luxi Sans" "Bold Oblique"

@calixteman
Copy link
Contributor Author

@jwillia3, what is the screenshot supposed to show ?
The tracemonkey.pdf file embeds all its fonts so it can't be used as a test case.
You should open the unix manual as in #17401 (comment) and see if the font used to render the page is the correct one.
If you're talking about the font used in the UI then it's another thing.

@jwillia3
Copy link

OK.
I didn't know you could change the preview file.
I still get Bitstream Vera Serif preferentially.
Screenshot 2024-01-15 at 08-40-10 unix_prog_design pdf

These are the steps I took.

  1. I download http://harmful.cat-v.org/cat-v/unix_prog_design.pdf.
  2. Open http://54.241.84.105:8877/91c07af1ae6be73/web/viewer.html
  3. Click the ">>" menu and select Open
  4. Open my downloaded copy of unix_prog_design.pdf
  5. See Bitstream Vera Serif is used
  6. Closed Firefox
  7. Add the following to fontconfig to disable Bitstream Vera Serif
<selectfont>
  <rejectfont>
    <glob>/usr/local/share/fonts/dejavu/*</glob>
  </rejectfont>
</selectfont>
  1. Repeat steps 1, 2, 3, and 4
  2. See Linux Libertine used

@calixteman
Copy link
Contributor Author

/botio-linux preview

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_preview from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/6263f2224aa9467/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/6263f2224aa9467/output.txt

Total script time: 1.42 mins

Published

@calixteman
Copy link
Contributor Author

@jwillia3, could you try again please ?

@jwillia3
Copy link

@calixteman Looks good. Thanks for looking into this.
Screenshot 2024-01-15 at 14-32-35 unix_prog_design pdf

Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch now does "a bunch" more than just adding a few new font-substitutions, so can you please extend the commit message to mention/explain the other changes?

Also, please run all tests!

@Snuffleupagus Snuffleupagus linked an issue Jan 16, 2024 that may be closed by this pull request
… Roman

and try to load the font family (guessed from the font name) before trying
the local substitution.
The local(...) command expects to have a real font name and not a predefined
substitution it's why we try the font family.
@calixteman
Copy link
Contributor Author

/botio test

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_test from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/1a217d36afc94ad/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_test from @calixteman received. Current queue size: 0

Live output at: http://54.193.163.58:8877/052730f4a13c7b4/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/1a217d36afc94ad/output.txt

Total script time: 24.42 mins

  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 269
  different first/second rendering: 2

Image differences available at: http://54.241.84.105:8877/1a217d36afc94ad/reftest-analyzer.html#web=eq.log

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/052730f4a13c7b4/output.txt

Total script time: 37.99 mins

  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 18
  different first/second rendering: 1

Image differences available at: http://54.193.163.58:8877/052730f4a13c7b4/reftest-analyzer.html#web=eq.log

@calixteman
Copy link
Contributor Author

The differences in the with the linux tests are expected since we now use a substitution as defined with in fontconfig files and not the ones we defined ourselves.

Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried looking through all of the Linux "failures", and as far as I can tell there's only small/expected movement however nothing seems outright broken.

r=me, thank you!

@calixteman calixteman merged commit e7d782e into mozilla:master Jan 16, 2024
9 checks passed
@calixteman
Copy link
Contributor Author

/botio makeref

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_makeref from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/ce93b34f364eb04/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_makeref from @calixteman received. Current queue size: 0

Live output at: http://54.193.163.58:8877/a0a3a8188d0fa19/output.txt

@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented Jan 16, 2024

@calixteman Once this reaches mozilla-central, we should check if it also fixes bug 1839860.

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/ce93b34f364eb04/output.txt

Total script time: 18.62 mins

  • Lint: Passed
  • Make references: Passed
  • Check references: Passed

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Success

Full output at http://54.193.163.58:8877/a0a3a8188d0fa19/output.txt

Total script time: 26.90 mins

  • Lint: Passed
  • Make references: Passed
  • Check references: Passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Poor Quality Font Substitution
4 participants