-
Notifications
You must be signed in to change notification settings - Fork 87
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
Use img to load avatars #311
base: master
Are you sure you want to change the base?
Conversation
da2x
commented
Dec 2, 2021
•
edited
Loading
edited
- Stop sending referrer request data to Gravatar (reduces data sharing with Gravatar, improves user privacy).
- Enable lazy loading avatar images (reduces image download request priority, defers downloading off-screen images).
- Remove the image from screen narrations (maintains current behavior).
- The change allows for sites to use stricter content security policies.
* Stop sending referrer request data to Gravatar (reduces data sharing with Gravatar, improves user privacy). * Enable lazy loading avatar images (reduces image download request priority, defers downloading off-screen images).
I appreciate this commit, it would solve two problems, security and accessibility. Unfortunately, I don't have time to look into this fully right now. I will soon, though. The changes look good, the only change I would make is to add a value for the HashOver used to use images for the avatars, and I would prefer to use images myself as well, but they were removed because they caused considerable performance degradation. You can read the changelog about this from mid 2015: changelog:L1264-L1269. At that time lazy loading was probably not well supported, I would be interested to know if it would solve the performance issues. If you could test your changes with 1,000 comments vs. using background images, with and without an ad-blocker, and verify that lazy loading the avatars doesn't impact performance, I will gladly accept your commit. |
This is due to the layout shifts caused by missing height and width attributes. It would need to load the image to determine its sizes. As long as it’s set in CSS (or directly on the element) it shouldn’t cause layout shifts. The default theme sets the attributes in CSS, so all is fine. I added it to the element too via c9e3d84. If a content blocker blocks Gravatar then it’s one on the network request level, so it doesn’t matter if it’s an image or a background element. Some older adblockers added To speed up performance in complex documents (lots of comments), then this should help tremendously (explainer video — TL;DR: the browser can cheat and assume the height of elements and doesn’t need to render them if they’re far off screen until you scroll down to them):
|