Bug 189230 - Shipped PNGs include bad profiles: iCCP: known incorrect sRGB profile
Summary: Shipped PNGs include bad profiles: iCCP: known incorrect sRGB profile
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Don Olmstead
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-09-02 04:14 PDT by fwdyi
Modified: 2018-11-12 14:51 PST (History)
8 users (show)

See Also:


Attachments
PNG run through zopflipng (501 bytes, image/png)
2018-10-22 13:13 PDT, Don Olmstead
no flags Details
Patch (357.60 KB, patch)
2018-10-24 12:37 PDT, Don Olmstead
no flags Details | Formatted Diff | Diff
Patch (357.60 KB, patch)
2018-10-24 12:42 PDT, Don Olmstead
joepeck: review+
Details | Formatted Diff | Diff
Patch (5.08 MB, patch)
2018-11-11 19:28 PST, Don Olmstead
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description fwdyi 2018-09-02 04:14:15 PDT
There is bad metadata embedded in some of the PNG icons shipped with WebKit.

These lead to warnings being printed to the terminal as libpng complains about them: "iCCP: known incorrect sRGB profile"

This affects software using WebKit as library as well.

https://stackoverflow.com/questions/22745076/libpng-warning-iccp-known-incorrect-srgb-profile has a nice overview and proposed fixes for removing this bad metadata.

This is a list of the affected files:

  ./WebInspectorUI.framework/Versions/A/Resources/Images/SliderThumbPressed@2x.png
  ./WebInspectorUI.framework/Versions/A/Resources/Images/SliderThumbPressed.png
  ./WebInspectorUI.framework/Versions/A/Resources/Images/SliderThumb@2x.png
  ./WebInspectorUI.framework/Versions/A/Resources/Images/SliderThumb.png
  ./WebInspectorUI.framework/Versions/A/Resources/Images/HoverMenuButton@2x.png
  ./WebInspectorUI.framework/Versions/A/Resources/Images/HoverMenuButton.png
  ./WebInspectorUI.framework/Versions/A/Resources/Images/GradientStopSelected@2x.png
  ./WebInspectorUI.framework/Versions/A/Resources/Images/GradientStopSelected.png
  ./WebInspectorUI.framework/Versions/A/Resources/Images/GradientStop@2x.png
  ./WebInspectorUI.framework/Versions/A/Resources/Images/GradientStop.png
  ./WebInspectorUI.framework/Versions/A/Resources/Images/DatabaseTable.png
  ./WebInspectorUI.framework/Versions/A/Resources/Images/ColorIcon@2x.png
  ./WebInspectorUI.framework/Versions/A/Resources/Images/ColorIcon.png
  ./WebCore.framework/Versions/A/Resources/textAreaResizeCorner@2x.png
  ./WebCore.framework/Versions/A/Resources/modern-media-controls/images/airplay-placard@2x.png
  ./WebCore.framework/Versions/A/Resources/modern-media-controls/images/airplay-placard@1x.png
  ./WebCore.framework/Versions/A/Resources/AttachmentPlaceholder@2x.png
  ./WebCore.framework/Versions/A/Resources/AttachmentPlaceholder.png

Please remove the buggy profiles so that unnecessary warnings are not printed to users and developers.
Comment 1 Radar WebKit Bug Importer 2018-09-02 07:23:52 PDT
<rdar://problem/44050379>
Comment 2 Brian Burg 2018-09-04 09:10:18 PDT
Would you be interested in writing a patch? I don't have a way to test this easily myself, as the Mac port does not use libpng this way (AFAIK).
Comment 3 John Friend 2018-10-13 11:21:53 PDT
I chased the same bug down and arrived here to report it.
The fix simply requires running imagemagick on the filed as described
in the SO question: "mogrify foo.png".

You can verify the fix by grepping the (binary) file for the chunk tag
"iCCP". After repairing the files grep should report a non-match.

I would submit a patch, but the tree snapshot is 1.8GB compressed (!) and that's too much to ask.

Since the files have been there for 7 years, and libpng1.6 has been yelling for 5, this may seem like a minor nuisance. But my system's log files are littered with these messages, so it *would* be great to fix this.
Comment 4 John Friend 2018-10-13 11:43:52 PDT
Reported to downstream QWebKit as well:

https://bugreports.qt.io/browse/QTBUG-71134
Comment 5 John Friend 2018-10-21 15:06:34 PDT
Please respond. This is a trivial fix for a problem which has been ongoing for years.
Comment 6 John Friend 2018-10-22 12:46:22 PDT
I've been advised by Kevin Kofler on the Redhat bugzilla that

pngcrush -ow -fix file.png

(overwrite file in place, fix)

is a better choice, since it strips the bad tag without altering the image data,
whereas mogrify recompresses it. I've verified that running the command twice on the same file doesn't alter the file hash.
Comment 7 Don Olmstead 2018-10-22 13:13:16 PDT
Created attachment 352904 [details]
PNG run through zopflipng

Here's a png run through zopflipng https://github.com/google/zopfli

I have all the pngs with this locally and am happy to make a patch. Please confirm that it works and I'll make a patch for it all.
Comment 8 John Friend 2018-10-23 15:07:11 PDT
Hi Don, 

I can confirm that libpng no longer produces the error. As I mentioned above, using "pngcrush -ow -fix" removes the bad chunk without recompressing the image data, so using that would be even better.
Comment 9 Don Olmstead 2018-10-24 12:37:23 PDT
Created attachment 353048 [details]
Patch

Runs all pngs through zopflipng.
Comment 10 Don Olmstead 2018-10-24 12:38:59 PDT
I'm guessing the ruby errors in the review patch might be due to this original issue.
Comment 11 Don Olmstead 2018-10-24 12:42:18 PDT
Created attachment 353049 [details]
Patch
Comment 12 Don Olmstead 2018-10-24 13:05:22 PDT
Comment on attachment 353049 [details]
Patch

Forgot a folder...
Comment 13 Joseph Pecoraro 2018-10-24 13:28:32 PDT
Comment on attachment 353049 [details]
Patch

r=me, Looks good on my Mac and the file sizes are dramatically better.

However this misses a few of the pngs that were called out in the bug description. Namely the ones in:

    Source/WebCore/Resources/*.png

This should modify resources like textAreaResizeCorner.png.
Comment 14 Don Olmstead 2018-10-24 13:32:04 PDT
(In reply to Joseph Pecoraro from comment #13)
> Comment on attachment 353049 [details]
> Patch
> 
> r=me, Looks good on my Mac and the file sizes are dramatically better.
> 
> However this misses a few of the pngs that were called out in the bug
> description. Namely the ones in:
> 
>     Source/WebCore/Resources/*.png
> 
> This should modify resources like textAreaResizeCorner.png.

Yea since I'm here I'll just make a script that runs through inside Source but not in Source/ThirdParty and compresses it.
Comment 15 John Friend 2018-10-24 17:53:31 PDT
For future issue, same might apply to PNG data embedded in CSS files. 

$ find -print0 -path '*.css' | xargs -0 grep "data:image/png"
Comment 16 John Friend 2018-10-31 02:27:06 PDT
bump.
Comment 17 Joseph Pecoraro 2018-11-05 11:24:09 PST
Don, are you able to put up another updated patch, or maybe you'll return to this after your other upstreaming work?
Comment 18 Don Olmstead 2018-11-05 11:46:00 PST
(In reply to Joseph Pecoraro from comment #17)
> Don, are you able to put up another updated patch, or maybe you'll return to
> this after your other upstreaming work?

Yea I'll get that going after I'm done with a couple patches I have in the pipe.
Comment 19 John Friend 2018-11-11 16:54:49 PST
bump.
Comment 20 Don Olmstead 2018-11-11 19:28:08 PST
Created attachment 354526 [details]
Patch
Comment 21 Don Olmstead 2018-11-11 19:31:28 PST
Comment on attachment 354526 [details]
Patch

This runs through everything. If I went too far with it let me know Joe.
Comment 22 Joseph Pecoraro 2018-11-12 13:07:08 PST
Comment on attachment 354526 [details]
Patch

r=me
Comment 23 WebKit Commit Bot 2018-11-12 13:44:31 PST
Comment on attachment 354526 [details]
Patch

Clearing flags on attachment: 354526

Committed r238106: <https://trac.webkit.org/changeset/238106>
Comment 24 WebKit Commit Bot 2018-11-12 13:44:33 PST
All reviewed patches have been landed.  Closing bug.
Comment 25 John Friend 2018-11-12 14:51:52 PST
Thanks for fixing.