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

Feature request: Set font-display rule to block to make Lighthouse happy #16077

Closed
3 tasks done
tagliala opened this issue Jan 15, 2020 · 21 comments
Closed
3 tasks done
Assignees
Milestone

Comments

@tagliala
Copy link
Member

tagliala commented Jan 15, 2020

Is your feature request related to a problem? Please describe.
There is a problem when validating Font Awesome websites using the css. In FA 5.7.0, we have switched to font-display: auto to allow users to use (the wrong) swap value, but now we have the chance to use font-display: block and get rid of the warning

Describe the solution you'd like
If we set the default font-display to block, as agreed in #14387 (comment), we can pass the validation without breaking stuff and fix rendering on browsers that do not honor the "similar to the block value" behaviour.

Describe alternatives you've considered
Ask users to change the scss files and recompile on our behalf, or override the css

Additional context
Ref:

Feature request checklist

  • This is a single feature (i.e. not a re-write of all of Font Awesome)
  • The title starts with "Feature request: " and is followed by a clear feature name (Ex: Feature request: moar cowbell)
  • I have searched for existing issues and to the best of my knowledge this is not a duplicate
@deLisle
Copy link

deLisle commented Feb 27, 2020

Looking forward to getting this fixed!

@guix77
Copy link

guix77 commented Feb 29, 2020

Following this

@robmadole
Copy link
Member

I've got this on the schedule for 5.12.2. I don't have a release date right now but it will go out with the next version of FA.

@tagliala tagliala added this to the 5.12.2 milestone Mar 4, 2020
@MDzyga
Copy link

MDzyga commented Mar 4, 2020

@robmadole
Could you release this no later than next Wednesday? My client really need this fix ASAP.

@robmadole
Copy link
Member

@MDzyga we can't make any commitments like that, sorry

@tagliala
Copy link
Member Author

tagliala commented Mar 4, 2020

@MDzyga

My client really need this fix ASAP.

You can customize the css and host it on your server

Anyway, in my understanding, this fix is just a convention to avoid the lighthouse warning, there are no performance benefits

Also FWIW you don't lose anything on the performance score for this showing up in the audit, and for fonts that are going to change to block it's really just an 'ignore this URL' type of signal anyhow, so feel free to ignore that LH result in the meantime.

Ref: GoogleChrome/lighthouse#10127 (comment) (by Lighthouse's creator)

@guix77
Copy link

guix77 commented Mar 4, 2020

@MDzyga Could you fix your attitude no later than next Wednesday ? I really need it ASAP.

@MDzyga
Copy link

MDzyga commented Mar 4, 2020

Sorry guys, my client has a bit big pressure on audit logs.
But I think I could fix it ;)

@robmadole
Copy link
Member

@MDzyga it never hurts to ask :) The reason that we can't commit to that is that we have a lot of other dependencies that are still up in the air for 5.12.2. While the fix for this is simple, it's just one piece of luggage on a large train with other bags.

@MDzyga
Copy link

MDzyga commented Mar 4, 2020

@robmadole I understand.
Do you know how can I set cache and change link rel on preload?
here is my issue with screenshots from lighthouse: #16271

@robmadole
Copy link
Member

@MDzyga I responded in that ticket. Please do not cross-post in other issues to try and get responses. That just clutters up everyone's email and irritates the people who are trying to provide help.

PikachuEXE added a commit to PikachuEXE/font-awesome-rails that referenced this issue Mar 20, 2020
Lighthouse is warning about it and
some browsers would use incorrect behaviour (swap) if `auto` or nothing provided
Issue on FA:
FortAwesome/Font-Awesome#16077

Change already scheduled for FA 5.12.2
FortAwesome/Font-Awesome#16077 (comment)
@tagliala
Copy link
Member Author

Hi, could you please check 5.13.0?

tagliala added a commit to tagliala/tagliala.github.io that referenced this issue Mar 24, 2020
@tagliala
Copy link
Member Author

I've checked myself, looks like it has been fixed

Green: https://tagliala.github.io/fa16077-513.html
Warning: https://tagliala.github.io/fa16077-512.html

Closing here, but free to comment if anything is still wrong

@oleksandrzaiats
Copy link

oleksandrzaiats commented Mar 14, 2021

I've checked myself, looks like it has been fixed

Green: https://tagliala.github.io/fa16077-513.html
Warning: https://tagliala.github.io/fa16077-512.html

Closing here, but free to comment if anything is still wrong

Hi!
It seems that the issue still persist when using Font Awesome Wordpress plugin (https://wordpress.org/plugins/font-awesome/). I am using that plugin (with the latest version of font - 5.15.2) and I get Leverage the font-display CSS feature... in the report from Lighthouse. This warning displays fonts from urls like https://use.fontawesome.com/releases/v5.15.2/webfonts/fa-** as affected.
I checked the code of that plugin and it seems like there is no font-display set. I did not find any issue tracking for that plugin so I decided to write here.
Is it possible to fix that or redirect me to the proper place where I should rise that issue? Thanks!

@tagliala
Copy link
Member Author

Hi, I'm not familiar with the official wordpress plugin.

This is the proper repo to report issues: https://github.com/FortAwesome/wordpress-fontawesome

I checked the code of that plugin and it seems like there is no font-display set. I did not find any issue tracking for that plugin so I decided to write here.

Before opening a new issue, please share your website so I can take a look

@oleksandrzaiats
Copy link

oleksandrzaiats commented Mar 15, 2021

@tagliala Nice! Thanks for the link.

@robmadole
Copy link
Member

@oleksandrzaiats we have font-display set in the CSS. You'll have to take this up with the Lighthouse team as it appears they want swap?

@oleksandrzaiats
Copy link

@robmadole I have looked through the WP plugin and did not manage to find any font-display there.
That plugin adds such css where there is no font-display specified:

@font-face {
font-family: "FontAwesome";
src: url("https://${license_subdomain}.fontawesome.com/releases/v${version}/webfonts/fa-brands-400.eot"),
		url("https://${license_subdomain}.fontawesome.com/releases/v${version}/webfonts/fa-brands-400.eot?#iefix") format("embedded-opentype"),
		url("https://${license_subdomain}.fontawesome.com/releases/v${version}/webfonts/fa-brands-400.woff2") format("woff2"),
		url("https://${license_subdomain}.fontawesome.com/releases/v${version}/webfonts/fa-brands-400.woff") format("woff"),
		url("https://${license_subdomain}.fontawesome.com/releases/v${version}/webfonts/fa-brands-400.ttf") format("truetype"),
		url("https://${license_subdomain}.fontawesome.com/releases/v${version}/webfonts/fa-brands-400.svg#fontawesome") format("svg");
}

@robmadole
Copy link
Member

@oleksandrzaiats ah thank you. I was able to find it with that. I will get a PR in the Wordpress plugin repo for that.

@oleksandrzaiats
Copy link

@robmadole Great, thanks! Actually I have create an issue there already FortAwesome/wordpress-fontawesome#108.
I think you know it, but just in case, there are multiple constructions (like I mentioned) in the plugin that should be fixed.

@ma-deldari
Copy link

I've checked myself, looks like it has been fixed

Green: https://tagliala.github.io/fa16077-513.html
Warning: https://tagliala.github.io/fa16077-512.html

Closing here, but free to comment if anything is still wrong

I update font Awesome 5.10 to 5.15 it work for me thanks alot

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

No branches or pull requests

7 participants