Skip to content

Simplify svg font expansion logic. #24066

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

Merged
merged 1 commit into from
Oct 3, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 20 additions & 36 deletions lib/matplotlib/backends/backend_svg.py
Original file line number Diff line number Diff line change
Expand Up @@ -1154,47 +1154,31 @@ def _draw_text_as_text(self, gc, x, y, s, prop, angle, ismath, mtext=None):
if weight != 400:
font_parts.append(f'{weight}')

def _format_font_name(fn):
normalize_names = {
'sans': 'sans-serif',
'sans serif': 'sans-serif'
}
# A generic font family. We need to do two things:
# 1. list all of the configured fonts with quoted names
# 2. append the generic name unquoted
def _normalize_sans(name):
return 'sans-serif' if name in ['sans', 'sans serif'] else name

def _expand_family_entry(fn):
fn = _normalize_sans(fn)
# prepend generic font families with all configured font names
if fn in fm.font_family_aliases:
# fix spelling of sans-serif
# we accept 3 ways CSS only supports 1
fn = normalize_names.get(fn, fn)
# get all of the font names and fix spelling of sans-serif
# if it comes back
aliases = [
normalize_names.get(_, _) for _ in
fm.FontManager._expand_aliases(fn)
]
# make sure the generic name appears at least once
# duplicate is OK, next layer will deduplicate
aliases.append(fn)

for a in aliases:
# generic font families must not be quoted
if a in fm.font_family_aliases:
yield a
# specific font families must be quoted
else:
yield repr(a)
# specific font families must be quoted
else:
yield repr(fn)

def _get_all_names(prop):
for f in prop.get_family():
yield from _format_font_name(f)
# (we accept 3 ways CSS only supports 1)
for name in fm.FontManager._expand_aliases(fn):
yield _normalize_sans(name)
# whether a generic name or a family name, it must appear at
# least once
yield fn

def _get_all_quoted_names(prop):
# only quote specific names, not generic names
return [name if name in fm.font_family_aliases else repr(name)
Copy link
Member

Choose a reason for hiding this comment

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

We have used repr() before and it seems to have worked, but technically repr() is not exactly quoting. Would both single and double quotes be ok? Unlikely, but in theory Python could change that in repr (or possibly more likely an interpreter other than CPython could use other quotes). Also, can we be sure we will never have byte strings here?

It's more defensive to f-string the quotes explicitly.

Copy link
Member

Choose a reason for hiding this comment

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

Do browsers / svg viewers / svg care about double vs single quotes?

Copy link
Member

Choose a reason for hiding this comment

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

I think we should be safe on byte strings though (I would say a byte string as a font name is a bug or at least un-defined behavior).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Single and double quotes are equally valid, and backslash-escaping quotes is valid as well: https://developer.mozilla.org/en-US/docs/Web/CSS/string

for entry in prop.get_family()
for name in _expand_family_entry(entry)]

font_parts.extend([
f'{_short_float_fmt(prop.get_size())}px',
# ensure quoting and expansion of font names
", ".join(dict.fromkeys(_get_all_names(prop)))
# ensure expansion, quoting, and dedupe of font names
", ".join(dict.fromkeys(_get_all_quoted_names(prop)))
])
style['font'] = ' '.join(font_parts)
if prop.get_stretch() != 'normal':
Expand Down