Making WordPress.org

Opened 3 years ago

Last modified 4 weeks ago

#5464 new enhancement

Plugin Directory: Prevent SVN uploads of animated banners and icons

Reported by: ipstenu's profile Ipstenu Owned by:
Milestone: Priority: normal
Component: Plugin Directory Keywords:
Cc:

Description

Related to #5171, animation in icons and banners is getting bonkers. It's not a11y friendly at all, and it's annoying to everyone else.

IMO the BEST solution would be to have SVN block all animated images named icon* or banner* from being uploaded, though that would require checking GIF, PNG, an SVG.

There are some other options. For example, we could actually allow gifs but set them to not animate.

Change History (32)

#2 @eatingrules
3 years ago

In Slack we discussed the ability to play/pause the loops, but the more I think about that, the more I think that's pretty silly in this context -- and it would also get in the way of being able to click on the thumbnail (in the repo search results, for example) to go to a plugin's full page.

I think it would be good if the system would allow uploads of animated images, but then only display the first frame. But if the system just flat-out rejects any animated image files, I think that's fine too. Plugin authors will learn quickly not to submit animated files either way! ;)

Thank you!

#3 follow-up: @dd32
3 years ago

There are four things that need to be accounted for here:

  1. Multi frame Gifs
  2. Animated PNGs
  3. The above uploaded as a .jpg
  4. SVGs with CSS transitions or <animate> tags

There's several options that I can think of..

  • For 1-3 is to proxy the image via something that strips the animation
  • For 1-3 upon plugin parse, detect multi-frame images and ignore them
  • For 4 (SVGs) SVGRoot.pauseAnimations() should work for the animation tags, but I don't think that'll affect any JS/CSS transitions..

One of the complexities here is that the ps.w.org cdn where these images are loaded from is just a caching code-less cdn based on plugins.svn.wordpress.org, so inserting something else in there between SVN and clients is not super ideal.

I guess that means that the ideal place to handle this is at plugin import time, but the limitations there is that it means you can't strip it back to being a single frame when it's detected to be animated without also making a commit to svn.

Detecting animated images and looking for animate within SVGs and ignoring them if present is probably the simplest option here.

#4 in reply to: ↑ 3 @dd32
3 years ago

Replying to dd32:

One of the complexities here is that the ps.w.org cdn where these images are loaded from is just a caching code-less cdn based on plugins.svn.wordpress.org, so inserting something else in there between SVN and clients is not super ideal.

If wanting to take that route, we could use the s.w.org cdn which is very similar, perhaps even just for icons that are detected as being animated or svg? eg how the https://s.w.org/plugins/geopattern-icon/hello-dolly.svg icons work.

#5 follow-up: @Ipstenu
3 years ago

I guess that means that the ideal place to handle this is at plugin import time, but the limitations there is that it means you can't strip it back to being a single frame when it's detected to be animated without also making a commit to svn.

Would that be, we can't tell without a commit of the image to SVN? Or would it check on any import of anything to SVN?

#6 in reply to: ↑ 5 @dd32
3 years ago

Replying to Ipstenu:

Would that be, we can't tell without a commit of the image to SVN? Or would it check on any import of anything to SVN?

The intention of it was, We'd check on any import from SVN (Manual or triggered by a commit), but with the limitation, that stripping it back from animated to non-animated would require an asset commit. Comment #4 however points out that that limitation only applies when using ps.w.org if it was switched to using s.w.org for those affected images it'd be fine.

This ticket was mentioned in Slack in #meta by tellyworth. View the logs.


3 years ago

#8 @dd32
3 years ago

So, I went on a bit of an implementation attempt today here..

I went with attempting to return animated assets like so:

https://s.w.org/plugins/remove-animation/$plugin/$animated_asset

That works great for animated Gifs & PNGs (even when saved as .jpg), but SVGs are quite a bit harder. For standard images, I'm just running them through imagecreatefromstring() and imagepng() to output the first frame.

I've found that appending the following to SVGs tends to work well as well:

<script> SVGRoot.pauseAnimations(); </script>
<style>* { animation: none !important; }</style>

The problem I ran into was that it increases the potential for Javascript execution on a WordPress.org domain, something that plugins.svn.wordpress.org & ps.w.org currently protects against, and so serving those SVGs via s.w.org is a bit too risky for me.

So.. I think the correct way to handle this is just to rip the bandaid off and block them.

I don't think blocking the SVN commit itself is worth it, just block it at the plugin directory import step.

Blocking images based on multi-frame is easy enough it seems. SVGs are more difficult, but we can probably just block on the file containing <script, animate, or animation. I'm sure there's a few ways around that though, but if someone can work around that, they deserve a ban hammer :)

This ticket was mentioned in PR #22 on WordPress/wordpress.org by dd32.


3 years ago
#9

  • Keywords has-patch added

Initial attempt at serving non-animated icons/banners.
No animation detection included yet.

Work in progress.

Trac: https://meta.trac.wordpress.org/ticket/5464

#10 @dd32
3 years ago

  • Keywords has-patch removed

Of course, as soon as I posted that, I realised that simply serving the SVG as Content-Disposition: attachment would probably work around the XSS issues, but I'm still not convinced that it's the right move.

I've uploaded a PR of that for later reference if that's the route that gets taken.

I still think that blocking the import of assets we don't want around is probably the better move.

This ticket was mentioned in Slack in #polyglots by dd32. View the logs.


3 years ago

This ticket was mentioned in Slack in #forums by dd32. View the logs.


3 years ago

#13 @joyously
3 years ago

The animations in the plugin directory are increasing. What does this need in order to be deployed?

This ticket was mentioned in Slack in #core-media by joyously. View the logs.


3 years ago

#15 follow-up: @alanfuller
2 years ago

Obviously stopping animations appears tricky. Just throwing this in, please shoot it down in animated flames. Why to plugin devs animate icons? To stand out in the search lists! So if the plugin search algorithm could detect animations and bury animated icons at the bottom, plugin devs would quickly revert to static.

This ticket was mentioned in Slack in #forums by ipstenu. View the logs.


2 years ago

#17 follow-up: @tradesouthwest@…
2 years ago

There is a way to stop svg by hacking the path:

path {
    animation: loop 0s linear infinite;
}

I use an Extension (FF, guessing there is same for Webkit) and I added w.org/plugins to my CSS table in the Extension. Of course this only works locally.

Personally the issue is in need of attention. NOt being one to say less is better but, maybe just plain do not allow animation in plugin assets. After all, they are in violation of WCAG guidlines { 2.3.3 Animation from Interactions (AAA) } and cause those with ADHD to not be able to use page.

See: https://wordpress.org/support/topic/gif-animation-interfere-with-a-users-ability-to-use-the-whole-page/#post-15401877

#18 in reply to: ↑ 17 @tradesouthwest@…
2 years ago

Last edited 2 years ago by tradesouthwest@… (previous) (diff)

#20 @dd32
22 months ago

#6312 was marked as a duplicate.

#21 @tradesouthwest@…
22 months ago

  • Type changed from defect (bug) to enhancement

Update: Was able to get author to remove animation by submitting a request on the plugin's support page. See https://wordpress.org/support/topic/remove-banner-gif-please/#post-15675720

Author was very responsive and easy about it.

Maybe start doing similar for other authors. Awareness is good medicine.

#22 @sabernhardt
20 months ago

WPMU DEV has replaced their plugins' animated thumbnail images with static PNGs, too.

#23 @tradesouthwest@…
20 months ago

Yeah for WPMU! Maybe a project for someone who would like to contribute to WP world and is maybe not a coder or is entry level could be to have them go through plugins repo and enter a Support Ticket entry into the plugin support page which would politely ask the author to remove the animated image.

Whenever I have time, I personally will do this. But it seems like the number of these creatures are increasing and not diminishing. Peace.

#24 @alanfuller
20 months ago

Nice request, although " Also they are not permitted on wp.org repo." and pointing to
this ticket is a bit of a stretch.

But if it was actually written in the developer guidelines or at least mentioned here https://developer.wordpress.org/plugins/wordpress-org/plugin-assets/#plugin-icons that they should / must not be animated it would at least give some formality to any 'user' request.


#25 @Ipstenu
20 months ago

But if it was actually written in the developer guidelines or at least mentioned here

As I have often said, making a guideline that is not easily enforceable means it will be violated all the time. Where as blocking it at the source obviates that need.

I am opposed to making a guideline I know will not be possible to enforce properly. We have enough of a headache with trademarks, and that's actually a legal thing!

This ticket was mentioned in Slack in #accessibility by sergey. View the logs.


12 months ago

#27 in reply to: ↑ 15 @Hareesh Pillai
10 months ago

This is worth exploring. Identifying the right form of incentive would help achieve the desired outcome.

Replying to alanfuller:

Obviously stopping animations appears tricky. Just throwing this in, please shoot it down in animated flames. Why to plugin devs animate icons? To stand out in the search lists! So if the plugin search algorithm could detect animations and bury animated icons at the bottom, plugin devs would quickly revert to static.

#28 @tradesouthwest@…
10 months ago

As to "trademarks"

It is no problem if the program contains trademarked images and names,
provided the trademark usage and requirements don't make it difficult
in practice to change the program and publish a modified
version. In other words, it has to be easy to find and remove the
trademarks, if and when the trademark conditions require this.

https://lists.gnu.org/archive/html/savannah-hackers/2004-11/msg00508.html
https://google.github.io/opencasebook/trademarks/

IF an asset icon HAS a trademark then it is WITHIN the icon and protected, on its own. BUT the icon having animation is NOT "protected" in trademark landscape as the _icon_ must follow guidelines.

Removing animation is a "rule" of WP.org repo and regardless of legal standing of any image or graphic.

#29 @Ipstenu
10 months ago

IF an asset icon HAS a trademark then it is WITHIN the icon and protected, on its own

Not sure what you're trying to argue here, but trademarked logos in images is a totally separate topic.

tl;dr "If it's a company's trademarked logo, do NOT use it in your banners or icons."

There is no 'trademark' or legal issue with not wanting to allow animated icons/banners. The only reason I mentioned is that I see little to no value in making "No animated icons" a _guideline_ before we have some way to block them.

A guideline that has no technical backup is just telling the plugins team to do more work, and sets up people to hunt down possible violations which again, more work.

This is one of those things better served by having the code to say "nuh uh!"

#30 @johnbillion
5 months ago

I noticed recently that WooCommerce now has an animated icon. As one of the most popular plugins in the repo, and one developed by Automattic, it sets a precedence that I would prefer is not followed by 59,000 other plugins trying to keep up.

I would be in favour of adding a new guideline stating that animations should not be used in icons and banners, at least that's something that could be actioned without any development work needed.

Last edited 5 months ago by johnbillion (previous) (diff)

#31 follow-up: @shooper
4 months ago

A PR was recently accepted to improve this within WordPress core by respecting the 'prefers-reduced-motion' setting. (thanks @stevejonesdev)

https://core.trac.wordpress.org/ticket/55723
https://github.com/WordPress/wordpress-develop/pull/5130

Could this be a solution in the plugin directory as well?

#32 in reply to: ↑ 31 @stevejonesdev
4 months ago

Yes, I think the code I used in core could be easily modified to work in the plugin directory as well.

Replying to shooper:

A PR was recently accepted to improve this within WordPress core by respecting the 'prefers-reduced-motion' setting. (thanks @stevejonesdev)

https://core.trac.wordpress.org/ticket/55723
https://github.com/WordPress/wordpress-develop/pull/5130

Could this be a solution in the plugin directory as well?

Note: See TracTickets for help on using tickets.