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

Fixes animated gifs incorrectly looping/not stopping on last frame #21999

Closed
wants to merge 1 commit into from

Conversation

staufman
Copy link
Contributor

@staufman staufman commented Oct 29, 2018

Currently, if you load an animated gif using the standard Image component, it will not correctly respect the loop count property found in the Netscape App Extension block of the file. The issues are as follows:

  1. If the App Extension isn't present, the animated gif loops indefinitely when it should not loop at all.
  2. If the App Extension is present, the animated gif loops one less time than it should.

The other issue is that once the looping completes, the image doesn't pause at the last frame but instead, loops back to the beginning of the animation e.g. frame 1.

The fix does a few things:

  1. If there is no App Extension present, the image doesn't loop at all
  2. If there is an App Extension present, it loops the correct amount of times. For instance, if the loop count is 1, it means the gif should loop once after it finishes playing, for a total of two total loops.
  3. Once the number of loops completes (assuming loop count isn't set to 0 which means infinite), the animation pauses on the last frame.

Test Plan:

The easiest way to verify this works is to take the attached images which are labeled with how many times an image should loop. The number of times the lips in the image come to the foreground of the animation should match how many loops the animation is set to play.

You can verify the behavior for animated gifs now matches most other animated gif implementations like the one found in Chrome.

giphy-no-loop-no-app-extension
No loop

giphy-loop-once
Loop once

giphy-loop-twice
Loop twice

giphy-loop-thrice
Loop thrice

giphy-forever
Loop forever

Release Notes:

[IOS] [BUGFIX] [Image] - Fixes an issue where animated gifs weren't looping correctly
[IOS] [BUGFIX] [Image] - Fixes an issue where animated gifs weren't stopping at the last frame

@staufman staufman requested a review from shergin as a code owner October 29, 2018 15:49
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 29, 2018
@matthargett
Copy link
Contributor

Did you verify this is only a problem in iOS and there's no fix needed on the Android side?

@staufman
Copy link
Contributor Author

staufman commented Nov 2, 2018

Good question! I thought I read in another thread that this had already been fixed in Android but I will try to verify this over the weekend. Stay tuned!

@staufman
Copy link
Contributor Author

@matthargett Confirmed this has already been fixed on Android. So this should fix up iOS.

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Dec 1, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

@staufman merged commit de759b9 into facebook:master.

@facebook facebook locked as resolved and limited conversation to collaborators Dec 1, 2018
@react-native-bot react-native-bot added the Merged This PR has been merged. label Dec 1, 2018
kelset pushed a commit that referenced this pull request Dec 12, 2018
…21999)

Summary:
Currently, if you load an animated gif using the standard `Image` component, it will not correctly respect the loop count property found in the Netscape App Extension block of the file. The issues are as follows:

1) If the App Extension isn't present, the animated gif loops indefinitely when it should not loop at all.
2) If the App Extension is present, the animated gif loops one less time than it should.

The other issue is that once the looping completes, the image doesn't pause at the last frame but instead, loops back to the beginning of the animation e.g. frame 1.

The fix does a few things:

1) If there is _no_ App Extension present, the image doesn't loop at all
2) If there _is_ an App Extension present, it loops the correct amount of times. For instance, if the loop count is 1, it means the gif should loop _once_ after it finishes playing, for a total of _two_ total loops.
3) Once the number of loops completes (assuming loop count isn't set to 0 which means infinite), the animation pauses on the last frame.
Pull Request resolved: #21999

Differential Revision: D13287005

Pulled By: hramos

fbshipit-source-id: f7210ad40e0e76c9ec454953b8a067569d3feaaa
grabbou pushed a commit that referenced this pull request Dec 17, 2018
…21999)

Summary:
Currently, if you load an animated gif using the standard `Image` component, it will not correctly respect the loop count property found in the Netscape App Extension block of the file. The issues are as follows:

1) If the App Extension isn't present, the animated gif loops indefinitely when it should not loop at all.
2) If the App Extension is present, the animated gif loops one less time than it should.

The other issue is that once the looping completes, the image doesn't pause at the last frame but instead, loops back to the beginning of the animation e.g. frame 1.

The fix does a few things:

1) If there is _no_ App Extension present, the image doesn't loop at all
2) If there _is_ an App Extension present, it loops the correct amount of times. For instance, if the loop count is 1, it means the gif should loop _once_ after it finishes playing, for a total of _two_ total loops.
3) Once the number of loops completes (assuming loop count isn't set to 0 which means infinite), the animation pauses on the last frame.
Pull Request resolved: #21999

Differential Revision: D13287005

Pulled By: hramos

fbshipit-source-id: f7210ad40e0e76c9ec454953b8a067569d3feaaa
@hramos hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 6, 2019
t-nanava pushed a commit to microsoft/react-native-macos that referenced this pull request Jun 17, 2019
…acebook#21999)

Summary:
Currently, if you load an animated gif using the standard `Image` component, it will not correctly respect the loop count property found in the Netscape App Extension block of the file. The issues are as follows:

1) If the App Extension isn't present, the animated gif loops indefinitely when it should not loop at all.
2) If the App Extension is present, the animated gif loops one less time than it should.

The other issue is that once the looping completes, the image doesn't pause at the last frame but instead, loops back to the beginning of the animation e.g. frame 1.

The fix does a few things:

1) If there is _no_ App Extension present, the image doesn't loop at all
2) If there _is_ an App Extension present, it loops the correct amount of times. For instance, if the loop count is 1, it means the gif should loop _once_ after it finishes playing, for a total of _two_ total loops.
3) Once the number of loops completes (assuming loop count isn't set to 0 which means infinite), the animation pauses on the last frame.
Pull Request resolved: facebook#21999

Differential Revision: D13287005

Pulled By: hramos

fbshipit-source-id: f7210ad40e0e76c9ec454953b8a067569d3feaaa
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API: Animated CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants