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

AccessKit Disable GIFs: Hide animated images instead of covering them #1707

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

marcustyphoon
Copy link
Collaborator

@marcustyphoon marcustyphoon commented Jan 22, 2025

Description

Right now, Disable GIFs hides most animated images when they aren't hovered by covering them with a non-animated one, and handles transparency by setting an opaque "white" background on the cover so the real, animated image can't be seen through transparent parts. This works great as long as the background is actually supposed to be "white" (which it is, in posts, even with Themed Posts). It doesn't work if one wants to apply that part of the Disable GIFs to other parts of the Tumblr UI which may have different background colors (in this case, the "recommended" element at the top of the Tumblr TV interface), and also might use a bit more CPU than the alternative.

What's the alternative? We simply hide the animated image when it's not hovered, just like we hide the cover when it is hovered. Thus, only one image is ever displayed at a time, and there's no need to implement anything special to make transparent images work.

This also means that we no longer care about the layer order at all, and in fact it's slightly more elegant due to the way CSS selectors work to put our canvas element before (i.e. what would have been "behind") the native animated image, similar to how Vanilla Videos inserts its replacement element before the native element and hides the native element with a ~ selector. removed for consistency with possible future development

(Aside: #1458, which I've done a reasonable amount of testing on, already uses the main technique used here. It will merge conflict with this, but it needs some refactoring anyway, hence its draft status.)

Testing steps

  • Confirm that Disable GIFs functions correctly on transparent GIFs, e.g. https://www.tumblr.com/animatedtext.
  • Confirm that Disable GIFs functions in Firefox and Chromium.
  • Confirm that there's no unusual delay/animation when moving the cursor onto or off of a paused GIF; there should be a seamless transition between the paused and animated images.
  • Open https://www.tumblr.com/search/gif in masonry view and confirm that small GIFs, such as the related tags in the sidebar and side-by-side GIFs in photosets, have small GIF labels.

@marcustyphoon marcustyphoon force-pushed the disable-gifs-no-opaque branch from 184ba7b to 6bc6630 Compare January 22, 2025 08:51
@marcustyphoon marcustyphoon changed the title Disable GIFs: Hide animated images instead of covering them AccessKit Disable GIFs: Hide animated images instead of covering them Jan 22, 2025
@marcustyphoon marcustyphoon removed the request for review from AprilSylph January 23, 2025 14:20
@marcustyphoon
Copy link
Collaborator Author

marcustyphoon commented Feb 8, 2025

Ooh, fascinating!

In Firefox, this causes caused a white flash when one hovered over an image for the first time in some circumstances, which I had always wondered about when developing #1458. Turns out there's an explanation: Firefox uses a default value of "async" for images' decoding property, which means that if we hide the real image element and do so quickly enough, the browser will download it but will not decode it until it becomes visible again.

This can be fixed completely cleanly by gifElement.decoding = 'sync'... or, to avoid making a DOM mutation, by just calling gifElement.decode() to instruct the browser to begin the decoding process even if the element is hidden!

Note that:

  • As I noted in Clarify image.decode() failure on lazy-loaded images mdn/content#38034, gifElement.decode() (currently?) fails in Firefox on loading="lazy" images that haven't loaded yet, so we need to fire it after waiting for gifElement.complete/gifElement.onload
  • gifElement.decode() returns a promise, but in this case we have no reason to actually wait for it; waiting for it would be a regression here as we only need the image's source value to continue with the pausing process.

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.

1 participant