55093 - When display is 32 bit depth, printing can fail.

Status: VERIFIED FIXED

(Core :: Printing: Output, defect, P3)

Description

[dcone (gone)]

Printing can fail if the display is in 32 bit depth and the printer does not 
support Device Dependent Bitmaps.  Also I have identified a possible leak which 
is also addressed in the patch.
I have found the problem.. and the patch is as follows:

the areas marked with // FIX  are the parts of the code added.


/** ----------------------------------------------------------------
 * Build A DIB header and allocate memory 
 * @update dc - 11/20/98
 * @return void
 */
nsresult
BuildDIB(LPBITMAPINFOHEADER  *aBHead,PRInt32 aWidth,PRInt32 aHeight,PRInt32 
aDepth,PRInt8 *aNumBytesPix)
{
PRInt16 numPaletteColors;

  if (8 == aDepth) {
    numPaletteColors = 256;
    *aNumBytesPix = 1;
  } else if (24 == aDepth) {
    numPaletteColors = 0;
    *aNumBytesPix = 3;
  } else if (16 == aDepth) {
    numPaletteColors = 0;
    *aNumBytesPix = 2;  
  } else if (32 == aDepth) {
    numPaletteColors = 0;
    *aNumBytesPix = 4;
  } else{
    NS_ASSERTION(PR_FALSE, "unexpected image depth");
    return NS_ERROR_UNEXPECTED;
  }

  // have to clean up the aBHead if it exists already
  if (nsnull != (*aBHead))
    delete[] (*aBHead);

  if (0 == numPaletteColors) {
    // space for the header only (no color table)
    (*aBHead) = (LPBITMAPINFOHEADER)new char[sizeof(BITMAPINFO)];
  } else {
    // Space for the header and the palette. Since we'll be using DIB_PAL_COLORS
    // the color table is an array of 16-bit unsigned integers that specify an
    // index into the currently realized logical palette
    (*aBHead) = (LPBITMAPINFOHEADER)new char[sizeof(BITMAPINFOHEADER) + (256 * 
sizeof(WORD))];
  }

  (*aBHead)->biSize = sizeof(BITMAPINFOHEADER);
  (*aBHead)->biWidth = aWidth;
  (*aBHead)->biHeight = aHeight;
  (*aBHead)->biPlanes = 1;
  (*aBHead)->biBitCount = (WORD)aDepth;
  (*aBHead)->biCompression = BI_RGB;
  (*aBHead)->biSizeImage = 0;            // not compressed, so we dont need this 
to be set
  (*aBHead)->biXPelsPerMeter = 0;
  (*aBHead)->biYPelsPerMeter = 0;
  (*aBHead)->biClrUsed = numPaletteColors;
  (*aBHead)->biClrImportant = numPaletteColors;

  return NS_OK;
}

Comment 1

[dcone (gone)]

Attachment: patch file

Comment 2

[dcone (gone)]

Attachment: This is the real patch..

Comment 3

[dcone (gone)]

*** Bug 57088 has been marked as a duplicate of this bug. ***

Comment 4

[buster]

aBHead is never checked for null after allocation.  could you also add that check?

I don't quite get the ownership model for aBHead.  If it's passed in non-null,
this routine is responsible for freeing that memory?  Sounds pretty kludgy.
What if this routine isn't called? Does the memory leaks?  What if the caller
passes in an uninitialized pointer rather than remembering to null out the
pointer first?  We'll crash!  If it's not too much work, I'd like to either see
the ownership model for this fixed, or at least documented.

sr=buster as is, but if you don't fix the above issues, please open a new bug on
them.

Comment 5

[Kevin McCluskey (gone)]

I agree with Steve. The ownership of LPBITMAPINFO data is confusing.

I think it would better to put the code to delete the LPBITMAPINFO data before
the call to BuildDIB in nsImageWin::ConvertDDBtoDIB, Instead of inside BuildDIB.
This way it will be clear that nsImageWin owns the LPBITMAPINFO
 data.

Comment 6

[dcone (gone)]

Attachment: Revised patch.. this patch also has a background fix in it.

Comment 7

[Kevin McCluskey (gone)]

Revised patch looks good.
r=[e-mail]

Comment 8

[dcone (gone)]

Checked in fix.. 

Comment 9

[shrirang khanzode]

v