This is a very recent regression and shows up if you run the test_nsIAccessibleImage.html chrome test file: not ok - wrong name for nonLinkedImageEmptyAlt! got null, expected "" not ok - wrong name for linkedImageEmptyAlt! got null, expected "" The empty alt attribute (alt="") tells screen readers that they should not repair the image's accessible name, but treat it as presentational. This was introduced with the last wave of checkins on Dec 3.

Pushed disabling of testing these two images: http://hg.mozilla.org/mozilla-central/rev/a2986966ab87. As part of the patch to fix this bug, the two commented-out lines should be reenabled.

Comment on attachment 352512 [details] [diff] [review] patch conditional r+ As per IRC chat it would be nice to change the comments to say something about an explicitly provided empty name. >- if (aName.IsVoid() && hasAltAttrib) { >+ if (aName.IsEmpty() && hasAltAttrib) { > // No accessible name but empty alt attribute is present. This means a name > // was provided by author and AT repair of the name isn't allowed. >- aName.Truncate(); >+ return NS_OK_EMPTY_NAME;

I mean't to add that the "AT repair" comment is confusing I think.

Comment on attachment 352512 [details] [diff] [review] patch Like David said, the comment should be improved. I suggest to merge the comment from the top of the method and the one within the "if" block and put it before the "if" block since this is where the extra processing begins. With that, r=me.

David and Marco, please look at new comments. Asking additional review from Aaron.

regression from bug 467055

I think it should probably be: (hasAltAttribute || hasAriaLabelAttribute || hasAriaLabelledByAttribute). In any of those cases the author has used *something* to indicate there is an empty accessible name. What do you guys think? If we do that then let's make sure the implementor's guide says that.

Yes, I think it's reasonable that any empty attribute used for name calculation should provide empty name if further algorithm doesn't provide not empty name.

Ok, you still have my r+, since that's an easy change to make to this patch.

(In reply to comment #10) > Ok, you still have my r+, since that's an easy change to make to this patch. I would prefer to have separate bug for this even the patch is simple.

Comment on attachment 352662 [details] [diff] [review] patch2 Two ortographical nits: >+ * Result can be string or null. String value means the name was specified and >+ * AT can't repair the name even if the string is empty (that means empty name >+ * was provided by author itentionally). In the case of null value AT are free "itentionally" should be "intentionally" (is missing the n). >+ // No accessible name but empty 'alt' attribute is present. This means >+ // an empty name was provided by author intentionally and image is >+ // presentational in the case if futher name computation algorithm don't >+ // provide non empty name (see nsIAccessible::name attribute for details). "futher" should be "further" (misssing r before the t), and "algorithm don't" should be "algorithm doesn't". Thanks ffor the changes, this makes things much clearer.

Comment on attachment 352662 [details] [diff] [review] patch2 I'd like to offer some commenting suggestions, building on what Marco said: >+ * Result can be string or null. String value means the name was specified and >+ * AT can't repair the name even if the string is empty (that means empty name >+ * was provided by author itentionally). In the case of null value AT are free >+ * to compute the name by own algorithms. Would the following comment work here? Value can be string or null. A null value indicates that AT may attempt to compute the name. Any string value, including the empty string, should be considered author-intentional, and respected. >+ if (aName.IsEmpty() && hasAltAttrib) { >+ // No accessible name but empty 'alt' attribute is present. This means >+ // an empty name was provided by author intentionally and image is >+ // presentational in the case if futher name computation algorithm don't >+ // provide non empty name (see nsIAccessible::name attribute for details). >+ return NS_OK_EMPTY_NAME; How about this wording? An empty alt attribute is used to indicate a decorative image; no further name computation required. Also I like Aaron's has* additions. BTW here's a related WCAG 2.0 statement: http://www.w3.org/TR/2008/NOTE-WCAG20-TECHS-20081211/H67

(In reply to comment #13) > (From update of attachment 352662 [details] [diff] [review]) > I'd like to offer some commenting suggestions, building on what Marco said: > > >+ * Result can be string or null. String value means the name was specified and > >+ * AT can't repair the name even if the string is empty (that means empty name > >+ * was provided by author itentionally). In the case of null value AT are free > >+ * to compute the name by own algorithms. > > Would the following comment work here? > > Value can be string or null. A null value indicates that AT may attempt to > compute the name. Any string value, including the empty string, should be > considered author-intentional, and respected. nice. > >+ if (aName.IsEmpty() && hasAltAttrib) { > >+ // No accessible name but empty 'alt' attribute is present. This means > >+ // an empty name was provided by author intentionally and image is > >+ // presentational in the case if futher name computation algorithm don't > >+ // provide non empty name (see nsIAccessible::name attribute for details). > >+ return NS_OK_EMPTY_NAME; > > How about this wording? > > An empty alt attribute is used to indicate a decorative image; no further name > computation required. it's opposite wording actually.

(In reply to comment #13) > Also I like Aaron's has* additions. I filed bug 469449

(In reply to comment #14) > > >+ if (aName.IsEmpty() && hasAltAttrib) { > > >+ // No accessible name but empty 'alt' attribute is present. This means > > >+ // an empty name was provided by author intentionally and image is > > >+ // presentational in the case if futher name computation algorithm don't > > >+ // provide non empty name (see nsIAccessible::name attribute for details). > > >+ return NS_OK_EMPTY_NAME; > > > > How about this wording? > > > > An empty alt attribute is used to indicate a decorative image; no further name > > computation required. > > it's opposite wording actually. I think the wording is equivalent, but I'm happy either way :)

That's not good :) I changed it a bit. Hope it's ok now. http://hg.mozilla.org/mozilla-central/rev/160661559c62