18267 - altdisplays for images, not handled correctly on all platforms

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect, P3)

Description

[Chris Petersen]

Build: Apprunner
Version: 1999110808
Platforms: Mac 8.6 and Linux Redhat 6.0.
Other Platforms: Works on Windows 98

Expected Results: The Alt text should appear in the browser window.

What I got: The progess indicator in the status area continues to cycle and
doesn't stop. The ALT text never is displayed.

Steps to reproduce:

1) Load the following url in Apprunner:

http://slip/projects/marvin/simple/img_bmp.html

2) Notice the Alt text in not displayed. The progess indicator in the status area
continues to cycle.

Comment 1

[Eli Goldberg]

Commencing genie invocation. (Version 2.0. ;)

Good catch!

Comment 2

[Eli Goldberg]

Ian, might you wish to reassign this bug appropriately? Thanks!

Comment 3

[Hixie (not reading bugmail)]

Erm... Could a Netscape person attach the relevant bitmap file to this bug,
then change the test file to point to the attached bmp file, and then attach
the test file as well? Or just copy the lot onto a public web site. Cheers!

Comment 4

[Eli Goldberg]

[deferring to petersen, as it's his bug.]

Comment 5

[Chris Petersen]

Attachment: A IMG test case that uses a bmp file as it's source

Comment 6

[Chris Petersen]

Ok, the test case has been enclosed. You can use any bmp file as the source to
reproduce the problem. Create a bmp file on your machine and update the SRC path
in the HTML file.

Comment 7

[Hixie (not reading bugmail)]

Well, the progress indicator doesn't keep cycling for me, but yep, the image
doesn't display itself. Looks like an image bug to me, so I'm putting elig
back in as QA. The alt text bit is working fine.

Are we supposed to support BMP images?

The problem also occurs on Win32, so marking cross-platform.

Comment 8

[Eli Goldberg]

There are no plans that I know of to support the Microsoft Windows-specific
bitmap format, or any of the other 100+ image formats in circulation beyond PNG,
GIF, JPEG and (to an extent for legacy purposes) XBM.

I have seen a single page which uses it (beyond PC Magazine test suites ;), so
I'm not aware of any customer value added by its inclusion.

cpratt wrote up a BMP enhancement request.

Comment 9

[Chris Petersen]

This issue is not that we should support bmp format directly. The problem was the
alt text wasn't appearing in the browser.If the browser can't support the file
format, it should display the alternative text. The alt text wasn't being
displayed on Mac or Linux builds but was displayed correctly on the Win32 build.

Comment 10

[Hixie (not reading bugmail)]

Right -- the problem seems to be that now, we are recognising BMP files, but
not supporting them, and are never saying "no" to the layout engine, so the
layout engine never decides to display the alt text.

Comment 11

[pnunn]

Actually, the imglib is failing in FirstWrite as it should
and the error condition is passed back up to
nsStreamListenerEvent::HandlePLEvent().

I'm not clear what should happen from there.
I'm cc'ing the netlib guys for edification.

Comment 12

[pnunn]

Changing summary to reflect bug behavior.
old summary:
Alt text is not displayed when the IMG SRC is set to a bmp file.

Comment 13

[pnunn]

warren:
I'm reassigning this to you, just so it gets
on your radar.

Tell me what should happen with
an error return from the imglib in
nsStreamListenerEvent::HandlePLEvent()
and reassign it back to me if I can
make the changes.

thnx,
p

Comment 14

[Warren Harris]

If you return an error from OnDataAvailable, you should get called back with
OnStopRequest and that error code. Is that not happening? Perhaps a better
thing to do is just clean up the image request at that point and mark that load
as done rather than waiting for another callback.

Comment 15

[pnunn]

verified NT behavior ok.
Now checking Mac & X.
-pn

Comment 16

[pnunn]

I'm working on this bug as current top priority,
but I don't think it needs to be m13. Moving to
m14 to get off the critical m13 rader.
-pn

Comment 17

[pnunn]

verified linux behaviour correct.
-pn

Comment 18

[leger]

Adding pp keyword

Comment 19

[leger]

Putting on PDT+ radar for beta1.

Comment 20

[pnunn]

Verified the code path from imglib to necko is ok.
After an error we do get an OnStopRequest.

Updating summary to reflect current status:
>Win works. Alt text shown for non supported image type.
>linux behaves strangely. The page is display in 2 different layouts.
Reloading the page causes the layouts to toggle back and forth.
(??cache issue?) On one layout, the altdisp displays as it should.
On the other, it does not.
>mac behaves strangely. The errant image doesn't have a altdisp.... Until
you go to another page. 
-pn

Comment 21

[pnunn]

This bug may be a duplicate of bug #26582. It is
at least dependent on the fix for bug# 26582.

Neeti and I are both looking at it.

Comment 22

[neeti]

Changing summary to happening on all platforms.

When we try to load http://jazz/users/pnunn/publish/abmp3.html, which has the 
following source. 
<html>
<body>

<img src="a.bmp" alt="a.bmp exists" width=200>
1111
<!-img src="hat.gif" alt="hat.gif exists" width=200>


</body>
</html>
it does not show the alternate text for the invalid image file a.bmp, the first 
time we load it and the page does not stop loading. The
other image in the file hat.gif displays itself properly. I find that when we 
get ImageConsumer::OnDataAvailable, imglib returns a error
of NS_ERROR_ABORT. 
However, we do not get an ImageConsumer::OnStopRequest for this a.bmp file. Now, 
if we hit return in the url bar, it causes the page
to stop loading, the ImageConsumer::OnStopRequest is called and we momentarily 
display the alternate text. 

It then reloads the page again, and then we may get the error case as above(no 
OnStopRequest called), or it may load the page
properly (i.e the alternate txt is displayed, in this case we call abort on the 
second entry to ImageConsumer::OnDataAvailable,
because the mInterrupted status has been set to true on the first entry.). 

Neeti

Comment 23

[Warren Harris]

Rick, This is the bug I mentioned. Neeti says that if you just load a page with 
a.bmp, you'll never get the OnStopRequest, until you try to go somewhere else. 
The next page loading will cause it to come in. 

Their test scenario is further complicated by multiple images on the page, but 
I think you can simplify it to one missing image.

Comment 24

[neeti]

The new test url http://jazz/users/pnunn/publish/abmp3.html contains only one 
missing image and no other images.

Neeti

Comment 25

[Warren Harris]

Removing "4 days?" from status whiteboard. Need absolute date.

Comment 26

[rpotts (gone)]

This turned out to be an interesting case where the socket transport could block
forever, if the consumer returned a failure in OnDataAvailable(...).

If the socket transport was blocked waiting for room in the pipe and the
consumer failed without consuming *all* the data, then the transport would
remain blocked and the page would never finish loading...

Comment 27

[rpotts (gone)]

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

Comment 28

[Simon Fraser [no longer active]]

rpotts: can you attach a patch to this bug? I'd like to try it.

Comment 29

[rpotts (gone)]

Attachment: Here is the patch...

Comment 30

[rpotts (gone)]

I've just checked the fix in.

Comment 31

[Eli Goldberg]

This looks fixed in the 3.2.00 AM builds on all 3 platforms; ALT text displays
throbber stops throbbing.

Comment 32

[Pulsebot]

Pushed by ffxbld-merge:
https://hg.mozilla.org/releases/mozilla-beta/rev/fe329716e365
[fenix] For https://github.com/mozilla-mobile/fenix/issues/18267: Updates preferences visibility depending on creditCardsStore