101910 - alt text is not displayed in image properties

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Roman Belenov]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.4) Gecko/20010913
BuildID:    2001091303

Image Properties (that can be obtained by right-clicking the image and choosing
"Properties") do not include the content of the ALT attribute with alternate
text. Adding it seems reasonable since 1) the attribute is indeed the property
of the image - it gives textual representation of its meaning 2) other ways of
obtaining it (looking for it in the page source or browsing "Images" tab of
"Page Info" dialog ) are much less straightforward 3) this way seems appropriate
for users who want to see the ALT text (e.g. to find some information that
author intentionally or unintentionally didn't include in the main text of the
page) while preventing bad habit of using the ALT attribute for visual
enhancement of the page (e.g. current practice of using it for displaying tooltips).


Reproducible: Always
Steps to Reproduce:
1. Open http:///www.sun.com/access page in Navigator
2. Right-click the image with the sun and select "Properties"


Actual Results:  ALT attribute is not available.


Expected Results:  ALT attribute should be shown (that says that the image shows
the sun shining through a doorway - not  a window or just a rectangular hole in
the wall - which is not quite deducible from the picture itself).

Comment 1

[Cormac F]

Looks like it should be a dupe, but I can't find one.
Marking NEW. 

Comment 2

[Jim Ley]

From WAI guidelines:
  
<URL:
http://www.w3.org/WAI/UA/WD-UAAG10-20010831/guidelines.html
>
"2.3 Render conditional content. (P1)"
and in "3" below that we have:
| If C is a summary, title, alternative, description, or expansion of another 
| piece of content D, provide access through at least one of the following 
| mechanisms: 
|
| (1a) render C in place of D; 
| (2a) render C in addition to D; 
| (3a) provide access to C by querying D. In this case, the user agent must 
| also alert the user, on a per-element basis, to the existence of C (so that 
| the user knows to query D); 
| (4a) allow the user to follow a link to C from the context of D. 

and

http://www.w3.org/WAI/UA/UAAG10-TECHS/guidelines.html#tech-conditional-content

"Allow users to choose more than one piece of conditional content at a
given time. For instance,users with low vision may want to view images
(even imperfectly) but require a text equivalent for the image; the
text may be rendered with a large font or as synthesized speech. "

(which I interpret along with other WAI UA suggestions about VIP that ALT being 
displayed at the same time as the image as in a tooltip can be useful for 
colour blind and people with poor vision, but who still want to see an image if 
possible.)

There are two threads in comp.infosystems.www.authoring.html on the subject 
aswell.

As it means that Mozilla is not conformant to the WAI UA guidelines  I think it 
needs raising from minor?

Comment 3

[Neil Marshall]

Attachment: Shows the Alt text in the image properties window

Comment 4

[Paul Chen]

Comment on attachment 53838 [details] [diff] [review]
Shows the Alt text in the image properties window

r=pchen, I just wish are dialogs wouldn't automatically size to content ;-)

Comment 5

[Neil Marshall]

Should I put the alt text in a textbox or something, so the dialog won't resize 
to the screen with really really long alt text?

At the moment it goes off the edge with no way of reading it all.

Comment 6

[John Morrison]

(unsolicited advice, but...) I would just 'crop="right"' the text (i.e., given
the relative rarity of very long alt text, and the trivial loss of 
information if it is cropped). 

But, that should be a <label> not a <text> since <text> is going away soon, no?

Comment 7

[Roman Belenov]

I guess that cropping alt text will be against the idea of accessibility; anyway
correct alt texts - those that describe the content of the image instead of just
entitling it - are usually long.

Comment 8

[Hixie (not reading bugmail)]

Why can't we give the dialog a max-width and let the alt text wrap if appropriate?

Comment 9

[Neil Marshall]

Would long filenames wrap?  They don't have any spaces and I believe the word 
wrapping doesn't cut up what it thinks is a url.

Ideally I think it would be best if all the values were in textboxes and 
textareas which were are read only.  Then they could be copied to the clipboard 
as well.

Comment 10

[Asa Dotzler [:asa]]

-> default assignee

Comment 11

[Peter Trudelle]

cc aaronl for accessibility input

Comment 12

[Aaron Leventhal]

It sounds like a good idea, and I have no problem with it.

Comment 13

[Neil Marshall]

Attachment: Adds alt to properties dialog

Patchmaker, Patchmaker, make me a Patch...

Okay, I modified the preferences dialog to display the alt text and I made all
of the items in the dialog <textbox>s instead of <label>s so you can now copy
and paste the text.  I had to remove the click ability from the links to do
this  (I didn't remove the openLink() function though).

Now on my computer it will only display properly if on line 464 in metadata.js
I add a return null;
Either my computer is messed or thats another bug.

I wanted to make the alt text <textbox> to be 3 lines tall but I couldn't
figure out how to do that, so at the moment it's just a normal textbox.

Comment 14

[Neil Marshall]

Attachment: Adds alt to properties dialog

Caught some messed up textboxes.

Comment 15

[Neil Marshall]

Can someone review this?

Comment 16

[timeless]

Comment on attachment 68432 [details] [diff] [review]
Adds alt to properties dialog 

it appears that in one place you use
+	   <textbox readonly="true" id="misc-title-text"/>
without value="" and in all other places you use value="". personally i prefer
the omission of the value as long as xul doesn't require it or have any whacky
inheritance rules. My memory and xulplanet both indicate that no such
whackiness exists, so i'd request that a newer patch omit the value="".

otherwise the patch looks fine (i'm not marking has-review because i haven't
tested this).

Comment 17

[neverlate]

Please, please, PLEASE bring back image alt tooltips!  These tooltips are
invaluable when bad graphic designers (but good web developers) put good alt
text on lousy images.  Thanks for listening.  --Haig Evans-Kavaldjian
([e-mail])

Comment 18

[timeless]

1. that's not going to happen. 2. that has nothing to do with this bug. 3. 
please speak to someone who cares (/dev/null, nul:, ..., perhaps news:)

Comment 19

[Neil Marshall]

Attachment: The value=""'s are removed

Okay, I redid the patch with the new patchmaker/directory structure and I
removed the value=""'s from the textbox tags.

Patchmaker outputted this error:
The CVS version of your patch may not work correctly.
Patch size is 6857 bytes

...because I added in the metadata.dtd which is located in the embed directory
(There are 2 metadata.dtd's).  I dunno if it should be in there but I added it
for good measure.

Comment 20

[Christian :Biesinger (don't email me, ping me on IRC)]

>...because I added in the metadata.dtd which is located in the embed directory
>(There are 2 metadata.dtd's).

No, there is only one metadata.dtd, at least in the CVS tree. Maybe it gets
copied to two locations in the distribution.

Comment 21

[Neil Marshall]

It turned out to be a bug in patchmaker.  Gerve fixed the bug.  I'll redo the 
patch without the extra stuff in it sometime soon.

Comment 22

[Neil Marshall]

Attachment: Updated patch

Can someone review this patch?

Comment 23

[Christian :Biesinger (don't email me, ping me on IRC)]

uh... could you style the textboxes like "-moz-appearance:none;
background:transparent; border:none;" so that they don't look like textboxes?

Comment 24

[Neil Marshall]

Attachment: Removed the borders

Okay, I removed the borders and I fixed up the image width and height display,
so "px" is now written beside the number as opposed to outside the textbox.

To remove the borders, I used the pageInfo.css which is already in the skin
directories.  Should I be using that code or should there be a metadata.css? 
(I used an existing one because I'm using patchmaker and I don't know if it can
add files)

Also, I think the window is a little too narrow.  Does anyone know how to
specify a minWidth?  I tried minWidth="300" in the <window> tag but it didn't
appear to have any affect.

Comment 25

[Neil Marshall]

Attachment: Now specifies a minwidth=""

I got the minwidth working (Case sensitive).

Could someone review this patch?  I think I've got everything in order now.

Comment 26

[Neil Marshall]

Attachment: The screenshot

Comment 27

[Neil Marshall]

Attachment: The patch

This patch fixes bug 101910 (this one), bug 77898, bug 89258, bug 89266 and it
will make bug 89261 invalid.

Comment 28

[Matthew T (active 1999-2002)]

Comment on attachment 82534 [details] [diff] [review]
The patch

>--- xpfe\browser\resources\content\metadata.xul.bak	Sun Nov  4 21:03:50 2001
>+++ xpfe\browser\resources\content\metadata.xul	Mon May  6 17:01:35 2002
>...
>-<window id="metadata"
>+<dialog id="metadata"

That's not right. It's an info window, not a dialog. What's
the difference?
*   It's non-modal, not modal.
*   It closes with accel+W or the close box in the title bar.
*   It doesn't have `OK', `Cancel', or similar buttons.
See <http://arstechnica.com/reviews/01q2/macos-x-final/images/get-info-1.jpg>
for an example.

>...
>+        buttons="accept"

So, that's not right either.

>+        minwidth="350"

That should be width=350 height=350. See bug 89258.
(Even hard-coding the Windows size would be better
than making the size variable.)

>...
>- </window>
>+    <separator/>
>+  </vbox>
>+  <separator class="groove"/>
>+</dialog>

See above.

>--- xpfe\browser\resources\locale\en-US\metadata.properties.bak	Tue May 22 17:15:32 2001
>+++ xpfe\browser\resources\locale\en-US\metadata.properties	Mon May  6 16:35:56 2002
>...
>\ No newline at end of file

Oops.

>--- xpfe\communicator\resources\content\nsContextMenu.js.bak	Sat Apr 27 14:47:48 2002
>+++ xpfe\communicator\resources\content\nsContextMenu.js	Mon May  6 16:04:51 2002
>...
>         window.openDialog(  "chrome://navigator/content/metadata.xul",
>                             "_blank",
>-                            "scrollbars,resizable,chrome,dialog=no",
>+                            "scrollbars,chrome,dialog=yes",
>...

See above.

Comment 29

[Neil Marshall]

So, you want the height of the dialog hard coded?

Comment 30

[Neil Marshall]

Attachment: Adjustments made

Okay, I made the adjustments.  Specified the width and height, made it a window
as opposed to a dialog (removed the last separator), and added the carrage
return to the end of the one file.  IMO it looks terrible and in some rare
cases all the data might not fit.

Comment 31

[rbs]

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

Comment 32

[rbs]

Is the alt text going to wrap so it can always be seen in full?

Comment 33

[gavin long]

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

Comment 34

[Wesha]

Build 2002051808: TITLE=...'s are now shown in Properties. but still no ALT=... :(

Please please fix?

Comment 35

[timeless]

Comment on attachment 82578 [details] [diff] [review]
Adjustments made

	 <row id="link-url">
+	   <separator orient="vertical"/>

+    <separator/>
+  </vbox>

kinda strange, no?

Comment 36

[Alexey Chernyak]

After all XUL rehawl, let's review the JS code a bit.

The alt= attribute in images is *required* by the HTML spec, it is not optional,
so we might as well get attention to it's absence, thus encouraging users to use it.

alt= is required by <img>. It is optional in <input> (since input is not always
an image), but the spect says it SHOULD be used in <input type="image">.
<object> does not have an alt= attribute since it specifies alternative content
inside it's body.

+        if ("alt" in img) {
+            setInfo("image-alt", img.alt);
+        } else {
+            setInfo("image-alt", "");
+        }

if ("alt" in img) is a wrong check in this case because every image element has
.alt property in DOM regardless of wether it was specified in the document or
not. So we should check for the attribute itself.

Keeping all of above in mind, I sugest changing this code to something along the
lines of:

if ((imgType == "img") || (imgType == "input")) {
    if (img.hasAttribute("alt")) {
        if (img.alt != "")
            setInfo("image-alt", img.alt);
    } else {
        setInfo("image-alt", "none!");
       
document.getElementById("image-alt-text").setAttribute("style","font-style:italic");
    }
} else {
    hideNode("image-alt");
}

The behaviour is following:
The property is always displayed in the window for <img> and <input> images and
never displayed for <object>.
If alt="foo" is present, "foo" is shown.
If alt="" is present, the shown property is empty.
If alt= is absent, "none!" is displayed in italics. The use of italics indicates
that the text was not generated by the document itself.

In this example "none!" should be replaced by a string from metadata.dtd.
It would be nice to use color:red for it as well, to get attention to this
error, since a simple exclamation mark just doesn't do it. But would that be
skin-friendly? Any thoughts on that?
Alternative is text-decoration:blink but I'm not sure if that will work in XUL.


<area> also requires alt= attribute. However that belongs to the Link Properties
section and we need a link-alt-text textbox and a similar JS code for it.


Also the code fails to get location for <object> images since <object> uses
data= instead of src= to specify the URL. So the code should be changed to:

if (imgType == "object") 
    setInfo("image-url", img.data);
else
    setInfo("image-url", img.src);

Comment 37

[Alexey Chernyak]

The code I provided does not show anything on image maps, because after
getImageForMap() is executed, the imgType stays unset, and the check fails.

Image maps can be ether <img> or <object> so we need a way to set imgType for
these before alt text code is executed.

Comment 38

[Neil Marshall]

Alexey, I'm on vacation and my development box is a couple thousand kilometers
away at the moment.  Feel free to fix up the patch.

Comment 39

[Neil Marshall]

Attachment: New properties window with alt text

Well, I hammered away at it and I think it's good enough to get r='d now.  So
if anyone feels like reviewing it, go ahead and lets try to get this into moz
once and for all.

I fixed the alt text on image maps and I took into account some of the
suggestions that have been made.  Here's a pretty picture of it
http://www.eightlines.com/neil/properties.gif

The alt text doesn't have multiline textboxes because bug 60159 and bug 53673

Comment 40

[Charles Manske]

Comment on attachment 99087 [details] [diff] [review]
New properties window with alt text

r=cmanske

Comment 41

[Alec Flett]

Comment on attachment 99087 [details] [diff] [review]
New properties window with alt text

sr=alecf

Comment 42

[rbs]

> The alt text doesn't have multiline textboxes because bug 60159 and bug 53673

Wasn't that possible to try some other element less buggy than a textbox for the
alt-text so that it wraps? E.g., wouldn't <description>...alt
text...</description> work?

Comment 43

[Alexey Chernyak]

1. There is no way to tell wether alt text was generated by the page or by the
browser, eg. alt="(Missing)" or alt="(Blank)". I have seen similar cases in
Mozilla's UI being done by italicizing the browser generated text, so I think
that's the way to handle this (if in doubt ask mpt). And no need for brackets.

2. Alt text absence should be emphasized, not just mentioned, as it is one of
the most frequently commited HTML errors. The simplest way would be adding an
exclamation mark to it.

3. What happens to alt text of <img> with an <area>? It stays inaccessible? As I
mentioned previously Image Properties of an area should contain image alt text,
not <area> alt text. Alt text of <area> should go into Link Properties section.



Also the new image size code needs a check for <object> images:

if (imgType == "object") 
    var size = getSize(img.data);
else
    var size = getSize(img.src);

Comment 44

[Alexey Chernyak]

Attachment: testcase

Try out your implementation on this testcase.
:o)

Comment 45

[Neil Marshall]

> wouldn't <description>...alt text...</description> work?

Sure I can use description, but can it be copied to the clipboard like a
textbox's text can?

> 1. There is no way to tell wether alt text was generated by the page or by the
> browser, eg. alt="(Missing)" or alt="(Blank)".

Yes I know, but metadata doesn't have it's own stylesheet and I'm using
patchmaker.  One of it's limitations is that you can't add new files.  I could
hijaak some other css file, but I wouldn't know which one to use.

> The simplest way would be adding an exclamation mark to it.
I didn't want to add an exclamation mark, that's why I put the brackets around
it instead.  I could have just as easily have copied the Page Info dialog and
just written "Not Specified".

> What happens to alt text of <img> with an <area>? It stays inaccessible?
Yes.  An image map can have multiple area tags with multiple alternate texts. 
What it does at the moment is, depending on where you clicked in the image map,
a different alternate text will be displayed.  If you select an area of the
image map without a hot spot defined, no alternate text is displayed.

> Also the new image size code needs a check for <object> images:
When I tried that, it didn't work.  Of course it may have had something to do
with the files being local.  I'm not sure.

Comment 46

[Alexey Chernyak]

> Yes I know, but metadata doesn't have it's own stylesheet and I'm using
> patchmaker.  One of it's limitations is that you can't add new files.  I could
> hijaak some other css file, but I wouldn't know which one to use.

You don't need a CSS file. You can change the styling dynamically through DOM.
I have provided working code previously:

document.getElementById("image-alt-text").setAttribute("style","font-style:italic");

> I didn't want to add an exclamation mark, that's why I put the brackets around
> it instead.  I could have just as easily have copied the Page Info dialog and
> just written "Not Specified".

You don't want to emphasize invalid code and thus nudge web developers to fix
it? The blind will thank you and raise a monument in your honour! Or you
couldn't care less? ;o)

> Yes.  An image map can have multiple area tags with multiple alternate texts. 
> What it does at the moment is, depending on where you clicked in the image map,
> a different alternate text will be displayed.

This is alternate text for links, not for image.

> If you select an area of the
> image map without a hot spot defined, no alternate text is displayed.

Did you check this with my testcase? Is this really what's happening with your
patch?

<img src="face.jpg" usemap="#map1" alt="You look at the face of a beautiful
woman." />
<map name="map1" id="map">
<area ... alt="See through her right eye." />
<area ... alt="See through her left eye." />
<area ... alt="Hear her speak." />
</map>

The Properties dialog becomes incomplete. The vital information is lost. Whos
eye? Who is speaking? The properties displayed for both <area> link and image
should be enough to understand what's happening here. This is the whole purpose
of alt text. With this patch it isn't. 

> When I tried that, it didn't work.  Of course it may have had something to do
> with the files being local.  I'm not sure.

Yes, it has to do with files being local. See the comment on getSize() function:
// Returns the size of the URL in bytes; must be cached and therefore an HTTP or
FTP URL

The check is still needed though.

Comment 47

[rbs]

> > wouldn't <description>...alt text...</description> work?
>
> Sure I can use description, but can it be copied to the clipboard like a
> textbox's text can?

Since the other existing properties cannot be copied, it seems to me that
wrapping is more important (the other alternative of cropping the text will
stand out to the eyes immediately and will leave something to be desired,
whereas copying requires extra steps from users and most users won't be doing that).

Comment 48

[Neil Marshall]

> Did you check this with my testcase? Is this really what's happening with your
> patch?

Yes I did check it.  It reveals a bug in image maps.

In the <img> image map example if you right click on the image map where there
is no hot spot, there is no way to select properties.  

This works:

  <object data="mozilla-banner.gif" type="image/gif" usemap="#map1"></object>
  <map name="map1">
   <area href="guide.html" 
          alt="Access Guide" 
          shape="rect" 
          coords="0,0,118,28">
  </map>

This doesn't:

  <img src="http://mozilla.org/images/mozilla-banner.gif" usemap="#map3" alt="" />
  <map name="map3" id="map3">
   <area href="/" shape="rect" coords="1,1,110,50" alt="" />
  </map>

Does anyone have the bug number for this one? 

As for getting all of the alt's to display in this dialog do you have any
suggestions on how to display more than one at a time?  The dialog is already
over 480 pixels tall in the most extreme cases.  Adding 2+ lines for alt text
could make it unusable for people at lower resolutions.  It's getting to the
point where it needs some interface change, such as tabs.  But that would be
another bug.



> Since the other existing properties cannot be copied, it seems to me that
> wrapping is more important

This patch changes all of the items so they are now textboxes and can be copied.
 If one of them isn't able to be copied it will also seem really odd. I changed
this to make it similar to property dialog's in windows and ie.  I have worked
with web developers who are constantly going into those dialogs and copying text
- that's why I felt the need to make the change.

...

In the mean time, I've got the italics in there (Required rewriting a section of
code) and I'm adding the image size of the data (Still working on it).  If
anyone comes up with a good idea on how to get the multiple alt's to display
nicely, fit in a 640x480 screen, and have the default "info" window width for a
mac, I'd like to hear it.

Comment 49

[Neil Marshall]

Attachment: New version

This version should work better with image maps.  It won't display all of the
alt texts at once for image maps, that can be the subject of another bug in the
future.  You can currently get at those alt texts if you select the correct
section of the image.

I added a null check to set info so the Link section should be a lot smaller in
most cases now.

Can people hammer away at it to make sure that my changes didn't break anything
else?

Comment 50

[Diethelm Schneider]

Since I use Mozilla, this browser was not able at any time to show 'alt'-text
for images. I tried Mozilla on Windows 98 and Windows 2000.
Actualy I use Mozilla 1.1.

Additional to the 'testcase' above you can use
http://selfhtml.teamone.de/html/grafiken/anzeige/img.htm

Diethelm

Comment 51

[Neil Marshall]

I found a couple bugs in the code so it's going to take yet more time to fix. :(

Comment 52

[Wesha]

Diethelm: see bug 25537

Comment 53

[Alexey Chernyak]

Your last patch was pretty much complete, except the image map issues. And those
can go into another bug.
Big thumbs up. You done a great job. Now get that bugfix patch out and let's
check the damn thing in. ;o)

(I'd still lose the brackets on (Missing) and (Blank) make them lower case and
add exclamation marks.)

Comment 54

[Neil Marshall]

Attachment: Fixes nulls

Let's do this one more time!
This new version fixes a problem with <a href="">'s and null values.
Once again, I need someone to r=/sr= this patch.

Comment 55

[Christian :Biesinger (don't email me, ping me on IRC)]

+    if (ismap == false && (imgType == "img" || imgType == "input")) {
+        setAlt(img);
+    } else if (ismap == false) {
+        hideNode("image-alt");
+    }

you're testing for "ismap == false" twice here. what about:

if (!ismap) {
    if ((imgType == "img" || imgType == "input"))
        setAlt(img);
    else
        hideNode("image-alt");
    }
}

Comment 56

[Neil Marshall]

Attachment: updated.

Fixed up the if statement.

Comment 57

[Neil Marshall]

Attachment: The correct patch this time.

Comment 58

[Charles Manske]

Comment on attachment 101729 [details] [diff] [review]
The correct patch this time.

r=cmanske

Comment 59

[jag (Peter Annema)]

Comment on attachment 101729 [details] [diff] [review]
The correct patch this time.

>+            if (img && img.localName.toLowerCase() == "img") {
>+                imgType = "img";
>+            } else if (img && img.localName.toLowerCase() == "object") {
>+                imgType = "object";
>+            }

var imgLocalName = img && img.localName.toLowerCase();
if (imgLocalName == "img" || imgLocalName == "object") {
    imgType = imgLocalName;
}


>+        var size;
>+        if ( imgType == "object" ) {
>+            setInfo("image-url", img.data);
>+            size = getSize(img.data);
>+        } else {
>+            setInfo("image-url", img.src);
>+            size = getSize(img.src);
>+        }

var imgInfo = imgType == "object" ? img.data : img.src;
setInfo ("image-url", imgInfo);
var size = getSize(imgInfo);


>@@ -327,14 +355,15 @@
>  */
> function setInfo(id, value)
> {
>-    if (value == "") {
>+    if (value == "" || value == null) {

Perhaps you could replace this with |if (!value)|

sr=jag

Comment 60

[Neil Marshall]

Attachment: Requested changes made (Comment #59)

Comment 61

[Alexey Chernyak]

> var imgInfo = imgType == "object" ? img.data : img.src;
> setInfo ("image-url", imgInfo);
> var size = getSize(imgInfo);

imgInfo is a rather misleading and confusing var name. imgURL would make code
more readable and understandable.

Comment 62

[Asa Dotzler [:asa]]

Comment on attachment 102604 [details] [diff] [review]
Requested changes made (Comment #59)

a=asa for checkin to 1.2beta (on behalf of drivers)

Comment 63

[Neil Marshall]

Fixed in the latest trunk build.

Comment 64

[Wesha]

verified on 2002101608.

Comment 65

[Tomas Ögren]

With this new stuff, text doesn't wrap which is a regression from before.
URLs, title properties etc doesn't fit.. There's room for like 40 chars in the
URL field for example..

Comment 66

[Brant Gurganus]

This patch introduced a regression that made links not clickable.  I have
supplied a patch in bug 1996 where I noticed this issue.

Comment 67

[Neil Marshall]

That was actually intentional on my part.  I made it so that you could copy the
text and it was placed in a textbox so that the dialog wouldn't become huge
based on the size of the url.

Does your new patch still allow you to easily copy the links by right clicking?