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

Added missing comma to data json for delay key #836

Conversation

regedarek
Copy link
Contributor

@regedarek regedarek commented Oct 12, 2017

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practices as demonstrated in the repository.

Description

I investigated this issue during upgrade of MiniMagick which forced me to use .data method instead of deprecated .details.

And ImageMagick returns invalid json for gif images.

Example json of gif file before my change.

    ....
    "dispose": "Undefined",
    "delay": "10x100"
    "compression": "LZW",
    "orientation": "Undefined",
    "properties": {
      "date:create": "2017-10-12T14:49:42+00:00",
      "date:modify": "2017-10-12T12:54:23+00:00",
      "signature": "36ab25b583a7e4359c232e8f3e2ee6f372e34d8ece465b1de86ac670dee134e4"
    }, ...

After my change

    ....
    "dispose": "Undefined",
    "delay": "10x100",
    "compression": "LZW",
    "orientation": "Undefined",
    "properties": {
      "date:create": "2017-10-12T14:49:42+00:00",
      "date:modify": "2017-10-12T12:54:23+00:00",
      "signature": "36ab25b583a7e4359c232e8f3e2ee6f372e34d8ece465b1de86ac670dee134e4"
    }, ...

Without this fix my application is unable to upload gif images, because I check first if they are valid.

Copy link

@raykov raykov left a comment

Choose a reason for hiding this comment

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

👍

@regedarek
Copy link
Contributor Author

regedarek commented Oct 12, 2017

I tested it already on docker with debian, lastest imagemagick. And it generates valid json.

@urban-warrior urban-warrior merged commit 76c7e83 into ImageMagick:master Oct 12, 2017
@regedarek
Copy link
Contributor Author

when do you plan to release it?

@regedarek
Copy link
Contributor Author

This is commit - which upgrade my career into another level!

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

Successfully merging this pull request may close these issues.

None yet

5 participants