1. Load http://cdn.techi.com/wp-content/uploads/2011/01/Google-Censorship-Graphic-01.jpg in Chrome 10.0.642.2 or above and notice the JPEG is corrupt. 2. Load http://cdn.techi.com/wp-content/uploads/2011/01/Google-Censorship-Graphic-01.jpg in Chrome 8.0.552.237 or below (or any other browser) and notice it displays properly. http://trac.webkit.org/changeset/73736 seems like a likely candidate.
(In reply to comment #0) > 1. Load http://cdn.techi.com/wp-content/uploads/2011/01/Google-Censorship-Graphic-01.jpg in Chrome 10.0.642.2 or above and notice the JPEG is corrupt. > 2. Load http://cdn.techi.com/wp-content/uploads/2011/01/Google-Censorship-Graphic-01.jpg in Chrome 8.0.552.237 or below (or any other browser) and notice it displays properly. > > http://trac.webkit.org/changeset/73736 seems like a likely candidate. In brief, libjpeg-turbo is not enabled on Chromium or Chrome by default because it causes a build break on Mac. (We can use it only when we explicitly set the 'use_libjpeg_turbo' value to 1 in '~/.gyp/include.gypi'.) By the way, to test the attached JPEG image with our continuous builds, r65032 (WebKit r71308) (*2) can show it without problems but r65179 (WebKit r71368) (*2) cannot. So, I assume a change from WebKit r71308 to WebKit r71368 caused this issue. (*1) <http://build.chromium.org/f/chromium/continuous/mac/2010-11-03/65032/chrome-mac.zip> (*2) <http://build.chromium.org/f/chromium/continuous/mac/2010-11-04/65179/chrome-mac.zip> Regards, Hironori Bono
Adam, is it possible this is a ICCJPEG bug? http://trac.webkit.org/changeset/71311/
Does this occur on platforms other than Mac?
(In reply to comment #3) > Does this occur on platforms other than Mac? I've only experienced it on mac.
It looks similar to problems we had with PNGs. In that case, we were trying to apply some image transformation (Grayscale => RGB) twice. Should be easy to narrow down by commenting out code.
Greetings Adam, (In reply to comment #5) > It looks similar to problems we had with PNGs. In that case, we were trying to apply some image transformation (Grayscale => RGB) twice. Should be easy to narrow down by commenting out code. Yes. This JPEG is a CMYK JPEG and JPEGDecoder applies CMYK->RGB conversion. (Unfortunately, I'm not sore who uses the ColorSpace object and applies another color conversion.) Is it OK to call 'ImageDecoder::setIgnoreGammaAndColorProfile(true)' also in this case as you did for Bug 49950 <http://webkit.org/b/49950>? Regards, Hironori Bono
Yes. That's probably the most expedient solution.
Greetings, (In reply to comment #7) > Yes. That's probably the most expedient solution. Thanks you for your advice. Even though I have implemented a fix, I cannot upload it because this 'Google-Censorship-Graphic-01.jpg' (used as a layout test) is too huge. Even though I will try creating a small CMYK JPEG at home (where I can use Photoshop), it would be definitely helpful to give me small CMYK JPEGs. Regards, Hironori Bono
Created attachment 81065 [details] CMYK JPEG
(In reply to comment #8) > it would be definitely helpful to give me small CMYK JPEGs. I've attached a smaller image (it should be green).
Created attachment 81194 [details] A copy-and-paste fix Greetings Mihai, (In reply to comment #10) > I've attached a smaller image (it should be green). Thank you so much for your test file. I have integrated your JPEG to my layout test. Regards, Hironori Bono
Comment on attachment 81194 [details] A copy-and-paste fix View in context: https://bugs.webkit.org/attachment.cgi?id=81194&action=review Nice test! > Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp:226 > + // Same as graysclale images, we convert CMYK images to RGBA graysclale => grayscale. Maybe "Same as with grayscale images" ? > LayoutTests/fast/images/cmyk-jpeg-with-color-profile.html:19 > Added: svn:eol-style > + LF We probably don't need this eol-style.
Created attachment 81196 [details] A copy-and-paste fix 2 (In reply to comment #12) Thank you for your quick review. I have updated this change to apply your comments and to clean up in my test script. > > + // Same as graysclale images, we convert CMYK images to RGBA > graysclale => grayscale. Maybe "Same as with grayscale images" ? Yes, it seems I have too focused on writing a test script to check my English. :) > > LayoutTests/fast/images/cmyk-jpeg-with-color-profile.html:19 > > Added: svn:eol-style > > + LF > > We probably don't need this eol-style. Thank you for noticing it. I have removed these properties from the newly-added files. Regards, Hironori Bono
Comment on attachment 81194 [details] A copy-and-paste fix View in context: https://bugs.webkit.org/attachment.cgi?id=81194&action=review > LayoutTests/fast/images/cmyk-jpeg-with-color-profile.html:10 > +<canvas id="canvas1" width="64" height="64"></canvas> If you create the canvas in the script, do you need to include it in the markup? > LayoutTests/fast/images/script-tests/cmyk-jpeg-with-color-profile.js:12 > +canvas.id = 'canvas1'; Don't need the ID. > LayoutTests/fast/images/script-tests/cmyk-jpeg-with-color-profile.js:24 > + // Read the pixels in the canvas and calcualte their avarage values. Typo ("calcualte").
Comment on attachment 81196 [details] A copy-and-paste fix 2 Looks good modulo Mihai's comments.
Created attachment 81332 [details] Patch for landing
Comment on attachment 81332 [details] Patch for landing Clearing flags on attachment: 81332 Committed r77724: <http://trac.webkit.org/changeset/77724>
All reviewed patches have been landed. Closing bug.
Greetings Mihai and Adam, Thank you so much for fixing my change and landing it. Regards, Hironori Bono