Image with no src doesn't show alt text instead

• Component: Forms
• Status: RESOLVED FIXED

[Kirby White]

An <input type="image" alt="test"> control doesn't show its alt text.  (It shows the alt if it has a src that can't be loaded, but not if it has no src at all.)

This looks related to 5566 and 5753, but not quite the same.

[Kirby White]

Created attachment 11864 [details]
Minimal test case

Note that
-<input alt="test" type="image" width="40" height="20">-
will show the alt text. This has got to do with the fact that WebKit only displays alt text if it fits into the image.

[Kirby White]

Created attachment 12232 [details]
Sets image size to fit alt text when no src is present

This fixes the problem for both <input type="image"> and <img> elements.

It also updates two other layout tests that were affected by the change.  I'm pretty sure the new expected result for the input-value-expected test is correct.  I don't know why the last few events in the focus2 test changed with the addition of a visible input element, but these new results seem more sensible than the previous set anyway (by not receiving keypresses on elements after they're blurred).

A question not addressed by this bug or patch is whether the alt text should also be shown when an image has a src that can't be loaded.  Currently we don't and IE does.

See bug 5566 comment #8.

[Kirby White]

(In reply to comment #4)
> See bug 5566 comment #8.

Cool, a spec.  I'm willing to take a swing at fixing the alt-text and error-icon handling for images next.

That spec <http://hixie.ch/specs/alttext> doesn't mention what to do when there's no src attribute at all, or equivalently(?) when the src = "".  I'd say that it's not really a broken or missing image, so it shouldn't show an error icon, but it should still show the alt text.

That's what this patch does.  It's not all that's needed for alt text, but it's partway there.

[Darin Adler]

Comment on attachment 12232 [details]
Sets image size to fit alt text when no src is present

+            imageObj->setImageSizeForAltText(NULL);

+                imageObj->setImageSizeForAltText(NULL);

Should be 0 rather than NULL. Might also just be good to have the parameter default to 0.

Need to indent 4 characters, not 2.

I'd like to see both the 4s and the 1024 and 256 as named constants, even though they weren't in the old code.

Is there any case where we have to transition from the missing image to a successful load. If so, then I don't see how the intrinsic width would be restored.

This otherwise looks great to me.

Not marking as reviewed because I'd like Hyatt to review this one if possible.

[Kirby White]

Created attachment 12338 [details]
patch with requested fixes

(In reply to comment #6)

> ...

Made all those changes.  I also found the problem with the missing last few events in the focus2.html test and fixed that (by adding another event to its test loops now that the image input element accepts focus too).

> Is there any case where we have to transition from the missing image to a
> successful load. If so, then I don't see how the intrinsic width would be
> restored.

I can't think of one, since it only sets the alt text size when there's no image even trying to be loaded, nor one already in the cache.  It works properly if a src is set in JS later, for instance.

[Darin Adler]

Comment on attachment 12338 [details]
patch with requested fixes

r=me

+   if (imageWidth != intrinsicWidth()) {

Need one more space here (indented wrong) -- looks like it got left on the line above instead.

[Kirby White]

Created attachment 12345 [details]
same patch, one more space

Thanks!

It doesn't seem like I really need separate review for this.  Whoever checks it in can either use the previous patch and add the space, or use this one.

[Mark Rowe (bdash)]

Landed in r18755.