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

PR: Add validation to raise exception when fonts are empty or corrupt #60

Merged
merged 7 commits into from
Dec 30, 2016

Conversation

dalthviz
Copy link
Member

This is part of an effort to improve the management of errors that cause a Spyder crash when trying to load fonts from the .ttf files in the fonts directory and these are corrupt.

@dalthviz dalthviz changed the title PR: Added md5 validation to raise exception when the fonts are corrupt. PR: Add md5 validation to raise exception when the fonts are corrupt. Dec 27, 2016
@dalthviz dalthviz changed the title PR: Add md5 validation to raise exception when the fonts are corrupt. PR: Add md5 validation to raise exception when the fonts are untrust/corrupt. Dec 28, 2016
@ccordoba12 ccordoba12 modified the milestones: v0.5, v0.4 Dec 30, 2016
@ccordoba12 ccordoba12 changed the title PR: Add md5 validation to raise exception when the fonts are untrust/corrupt. PR: Add validation to raise exception when the fonts are empty or corrupt Dec 30, 2016
if ttf_filename == 'fontawesome-webfont.ttf':
ttf_hash_code = 'a3de2170e4e9df77161ea5d3f31b2668'
elif ttf_filename == 'elusiveicons-webfont.ttf':
ttf_hash_code = '207966b04c032d5b873fd595a211582e'
Copy link
Member

Choose a reason for hiding this comment

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

Let's make this a dictionary for clarity, like this:

md5_hashes = {'fontawesome-webfont.ttf': 'a3de2170e4e9df77161ea5d3f31b2668',
              'elusiveicons-webfont.ttf': '207966b04c032d5b873fd595a211582e'}

ttf_hash_code = 'a3de2170e4e9df77161ea5d3f31b2668'
elif ttf_filename == 'elusiveicons-webfont.ttf':
ttf_hash_code = '207966b04c032d5b873fd595a211582e'
if ttf_hash_code != None:
Copy link
Member

Choose a reason for hiding this comment

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

If we use a dictionary for hashes then this has to be

ttf_hash = md5_hashes.get(ttf_filename, None)
if ttf_hash is not None:
    ...

And yes, please use ttf_hash instead of ttf_hash_code for this variable.

@ccordoba12
Copy link
Member

@dalthviz, this looks good to me. I just left a couple of comments to improve readability ;-)

Also, please merge with master to fix the Travis errors.


loadedFontFamilies = QFontDatabase.applicationFontFamilies(id_)

if(loadedFontFamilies):
self.fontname[prefix] = loadedFontFamilies[0]
else:
print('Font is empty')
raise FontError(u"Font is empty at: '{0}'".format(
Copy link
Member

Choose a reason for hiding this comment

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

We need to improve this error message for people that uses QtAwesome on Windows 10. Please rephrase it for something like

"Font at "{0}" appears to be empty. If you are on Windows 10, "
"please read https://support.microsoft.com/en-us/kb/3053676 to "
"know how to prevent Windows from blocking the fonts that come "
"with QtAwesome."

md5_hashes = {'fontawesome-webfont.ttf':
'a3de2170e4e9df77161ea5d3f31b2668',
'elusiveicons-webfont.ttf':
'207966b04c032d5b873fd595a211582e'}
Copy link
Member

Choose a reason for hiding this comment

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

Please format this better:

md5_hashes = {'fontawesome-webfont.ttf':
              'a3de2170e4e9df77161ea5d3f31b2668',
              'elusiveicons-webfont.ttf':
              '207966b04c032d5b873fd595a211582e'}

@ccordoba12
Copy link
Member

Thanks @dalthviz, merging now :-)

@ccordoba12 ccordoba12 changed the title PR: Add validation to raise exception when the fonts are empty or corrupt PR: Add validation to raise exception when fonts are empty or corrupt Dec 30, 2016
@ccordoba12 ccordoba12 merged commit 1853d3d into spyder-ide:master Dec 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants