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

Breaking change in 1.19.0 #446

Closed
garrows opened this issue Sep 17, 2015 · 7 comments
Closed

Breaking change in 1.19.0 #446

garrows opened this issue Sep 17, 2015 · 7 comments

Comments

@garrows
Copy link

garrows commented Sep 17, 2015

Hi,
Love the module but your latest change seems to have broken things for me.

I'm getting the error for several files.
improper image header-' @ error/png.c/ReadPNGImage/3921`

Downgrading to 1.18.1 fixed the problem for me.

@rwky
Copy link
Contributor

rwky commented Sep 17, 2015

When reporting bugs please include the version of graphicsmagick/imagemagick you're using (gm -version/convert -version) as well as the version of this module and copies of any images you're having problems with.

@garrows
Copy link
Author

garrows commented Sep 18, 2015

Apologies. I'm usually more useful but I was in a rush (and sadly still am)

ImageMagick 6.9.1-10 Q16 x86_64
GraphicsMagick 1.3.21 2015-02-28 Q8

Tried on node v4.0.0 and v0.10.40

I'll try to get you some sample code later on.

@rwky
Copy link
Contributor

rwky commented Sep 23, 2015

Can you see if this is fixed in 1.20.0?

@garrows
Copy link
Author

garrows commented Sep 23, 2015

That has fixed it for both 0.10 and 4.0. Thank you!

@adurrive
Copy link
Contributor

Hello @garrows,

I've looked at most of the problems caused by 1.19.0 and I've made a new pull request. Your issue however would likely fail again.

Currently, the module does not provide the image format of buffers and streams to gm, as you may expect when providing a filename. If you used something like gm(buffer, 'image.png'), the second argument is essentially ignored and gm has no idea the buffer is a PNG. In most cases, this is fine since gm is able to find the correct format automatically. But in some others, it simply makes buffers and streams unusable.

1.19.0 attempted to fix this and restore the expected behaviour. This means that an incorrect filename, which until now had no impact at all, would fail. The correct way to use the automatic format inference, however, remains: if you do not provide a filename, using something like gm(buffer), gm will analyse the file signature as it does today.

It would be great to look further at the problematic files you used and how you used them to rule out other unforeseen issues. Would you still have them?

@garrows
Copy link
Author

garrows commented Jan 28, 2016

That's unfortunate. If you do have to make the breaking change make sure its on a major version change though.

The module I was having trouble with is https://github.com/garrows/IMMP which has a few tests which I think were failing with 1.19.0. I hope this helps.

@adurrive
Copy link
Contributor

Thanks I think I've spotted the issue and it's a miss on my part. I'll fix it and let @rwky decide what to do with the pull request.

adurrive added a commit to adurrive/gm that referenced this issue Jan 28, 2016
adurrive added a commit to adurrive/gm that referenced this issue Jan 28, 2016
adurrive added a commit to adurrive/gm that referenced this issue Jan 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants