Bug 8749 - XBM rendered incorrectly as black on white
Summary: XBM rendered incorrectly as black on white
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P4 Trivial
Assignee: Nobody
URL: http://homepage.mac.com/gregharewood/...
Keywords:
Depends on:
Blocks:
 
Reported: 2006-05-05 00:08 PDT by Greg Harewood
Modified: 2008-02-24 22:38 PST (History)
2 users (show)

See Also:


Attachments
xbm images in Mosaic, Netscape, IE 5 for Mac (324.54 KB, image/jpeg)
2006-05-05 12:27 PDT, Greg Harewood
no flags Details
xbm image creation patch (1.73 KB, patch)
2008-02-16 10:01 PST, Michael Knaup
darin: review-
Details | Formatted Diff | Diff
xbm image creation patch (1.73 KB, patch)
2008-02-16 11:25 PST, Michael Knaup
darin: review+
Details | Formatted Diff | Diff
xbm image creation patch (11.42 KB, patch)
2008-02-17 08:19 PST, Michael Knaup
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Greg Harewood 2006-05-05 00:08:08 PDT
I cannot find a standard relating to this.  However, XBM images are rendered as black on while.  For compatibility with NCSA Mosaic where they were most used, they should actually be rendered using the current foreground and background colors.  (I noticed this when trying to use them as a way to provided rounded corners in a CSS3 border using current colors.)
Comment 1 Alexey Proskuryakov 2006-05-05 01:35:35 PDT
Reporter, could you please attach a test case, or link to it?
Comment 2 Greg Harewood 2006-05-05 12:24:43 PDT
Test document:   http://homepage.mac.com/gregharewood/tmp/examplebmp/simple.html

Revised description:  correct behavior, per netscape 4.7, is to show black on a transparent background.  However, I found more variation than I remembered - see screenshot - so there is little point in changing Safari's handling.
Comment 3 Greg Harewood 2006-05-05 12:27:38 PDT
Created attachment 8127 [details]
xbm images in Mosaic, Netscape, IE 5 for Mac
Comment 4 Alexey Proskuryakov 2006-05-05 12:48:56 PDT
Firefox also renders the test case with transparent backround, so I guess Safari should as well.
Comment 5 Michael Knaup 2008-02-16 10:01:01 PST
Created attachment 19156 [details]
xbm image creation patch

I tried to create a patch to make xpms rendered as they are rendered in Firefox
Comment 6 Darin Adler 2008-02-16 10:36:51 PST
Comment on attachment 19156 [details]
xbm image creation patch

Looks good to me.

For compatibility with 64-bit, instead of float it should be CGFloat.

This is really a workaround for a Radar bug: <rdar://problem/3922468> browsers treat XBM images as black on transparent background, ImageIO decodes as black on white

review- because this won't compile 64-bit.
Comment 7 Michael Knaup 2008-02-16 11:25:29 PST
Created attachment 19158 [details]
xbm image creation patch

Changed the patch to use CGFloat instead of float
Comment 8 Darin Adler 2008-02-16 19:08:30 PST
Comment on attachment 19158 [details]
xbm image creation patch

Two remaining problems:

    1) A tab character in the change log.
    2) This needs a regression test. I think we could do one with <canvas>.

I'm tempted to say r=me as-is, but (2) is a serious issue. I'll do review+ for now, but please look into creating a regression test.
Comment 9 Michael Knaup 2008-02-17 08:19:05 PST
Created attachment 19175 [details]
xbm image creation patch

Added a regression test and removed the tab
Comment 10 Darin Adler 2008-02-17 09:47:04 PST
Comment on attachment 19175 [details]
xbm image creation patch

r=me

Once our canvas has an operation for reading pixels, we should change this into a dumpAsText() test.
Comment 11 Robert Blaut 2008-02-20 03:32:17 PST
P5 is not used for WebKit bugs. [http://webkit.org/quality/bugpriorities.html]
Comment 12 Darin Adler 2008-02-24 22:38:37 PST
Committed revision 30556.

After landing this I realized that:

    1) We should take advantage of getImageData to turn this into a text test rather than a pixel test.
    2) We should move the test out of the canvas directory -- it's a test of image rendering not of the canvas.

But I suppose neither of those is urgent.