Don't show "Alternate text" under Element Properties and Page Info for standalone images

VERIFIED WORKSFORME

Status

--
trivial
VERIFIED WORKSFORME
17 years ago
10 years ago

People

(Reporter: niederstrasser, Assigned: db48x)

Tracking

Bug Flags:
blocking-aviary1.0PR -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(5 attachments, 3 obsolete attachments)

(Reporter)

Description

17 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.2b) Gecko/20021031
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.2b) Gecko/20021031

When an image is viewed directly within the browser, selecting "Properties" from
the context menu gives the above error message for the "Alternate text"
property.  This is incorrect, as the image displays correctly in the browser and
does not have errors.

Reproducible: Always

Steps to Reproduce:
1. Load an image into the browser.  Can either be through a remote URL or drag
and drop from a local file.
2. Open the context menu on the image and select "Properties"
3. Look at the text given for "Alternate text."

Actual Results:  
"Alternate text" says 'The image [image_name] cannot be displayed, because it
contains errors.'

Expected Results:  
Image Properties should either not show the entire Alternate Text line or it
should leave the value blank.

An image viewed by itself does not have any alt-text, since that is a property
that gets assigned by a web page.  Therefore, the displayed value for the
Alternate Text property should be blank.

This should not be a dupe of Bug 121084 [cache: Images requested twice leads to
error] because 1) the images are shown without any problems in the browser and
2) this bug also appears when viewing local images and should not affect the
cache.  This is reproducible with all cache settings under Advanced Preferences.

Finally, the properties for the image from _within_ a web page behave as
expected.  Alternate Text shows the correct alt-text.
Actually, that _is_ the alt text for that image.  This way _if_ the image were
to have errors and not be displayable the alt text would be displayed instead.

Granted, that's an implementation detail, and we should probably not be showing
it in properties....

Comment 2

17 years ago
Confirming, changing summary a bit.  It's not the "Alternative Text" we need
changed but the fact we show it in the properties window in this case. 
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: In the Properties pop-up for an image, Alt Text has "The Image [image_name] cannot be displayed, because it contains errors." → Don't show "Alternate text" under properties for images loaded directly

Comment 3

17 years ago
Sending to ImageLib, since I need help assigning a particular imaging bug, and
they'll accept it in ImageLib and reassign it for me :-) Also changing HW/OS to
All/All, this happens on windows too (build 2002110708).
Assignee: asa → pavlov
Component: Browser-General → ImageLib
OS: MacOS X → All
QA Contact: asa → tpreston
Hardware: Macintosh → All
Eventually we'll fix this by using <object> instead of <img>, so this will
become a non-issue, so let's not worry about it...
Whiteboard: WONTFIX?
*** Bug 182191 has been marked as a duplicate of this bug. ***

Comment 6

16 years ago
*** Bug 184200 has been marked as a duplicate of this bug. ***

Comment 7

16 years ago
Why does properties show a filename fragment instead of a complete filename as
the "alt" text?
it shows the alternate text attribute, not the filename. They may be similar on
certain pages...

Comment 9

16 years ago
I was referring to an image file loaded directly from local storage, as the
summary describes. How can it have any attributes besides name, size, timestamp,
etc? I thought an alt attribute exists only when assigned by markup code, which
is bypassed using a file picker to load an image file.

Comment 10

16 years ago
I see this as well, go load an image directly in the browser, right click it and
hit properties.  The Alternate text attribute should be blank!

Also, if you drag the image off to the desktop, for example, you'll get a
shortcut with the filename: The image [image_name] cannot be displayed, because it
contains errors..url
Oh, you are just repeating the bug report.

Yes, we know.

Thanks for confirming it though. :-)
*** Bug 187726 has been marked as a duplicate of this bug. ***

Comment 13

16 years ago
*** Bug 188068 has been marked as a duplicate of this bug. ***
*** Bug 198847 has been marked as a duplicate of this bug. ***

Comment 15

16 years ago
Could we change the summary from "Don't show "Alternate text" under properties
for images loaded directly" to "Don't show "Alternate text" under element
properties for images loaded directly"? I tried searching for this bug using
"element properties" and ended up submitting a duplicate.

Comment 16

16 years ago
Summary changed as requested in comment 15.
Summary: Don't show "Alternate text" under properties for images loaded directly → Don't show "Alternate text" under element properties for images loaded directly

Comment 17

16 years ago
Changing summary back. It's not okay to not show the alt text. We use the ALT
text for screen reader accessibility.
Severity: normal → major
Keywords: access, sec508, topembed
Summary: Don't show "Alternate text" under element properties for images loaded directly → Alt Text has "The Image [image_name] cannot be displayed, because it contains errors."

Comment 18

16 years ago
Nevermind, I'm going to deal with screen reader stuff on my end. I'll have to
circumvent this fake alt text by detecting the image document.
Severity: major → normal
Keywords: access, sec508, topembed

Comment 19

16 years ago
aaronl, if a user loads http://example.org/ting.gif how do you plan to get an
ALT text from it, for accessibility, or any other admirable purpose ?

Why has this bug's summary changed from the remarkably clear "don't do X when Y
is true" to "X is broken" ?

This bug has nothing to do with accessibility, or ALT text for that matter. The
element properties window is wrong to try to show "Alternate text".
Summary: Alt Text has "The Image [image_name] cannot be displayed, because it contains errors." → Don't show "Alternate text" under properties for images loaded directly
(Reporter)

Comment 20

16 years ago
As the reporter, I'm changing the summary back to the description of the problem.  

This bug is not about hiding the ALT text from browsers (graphical/screen
readers/etc).  This bug is about incorrectly showing an internal Mozilla
ALT-text string (used for when a directly loaded image is broken) in the
"Element Properties" dialog (see comment #1).
Summary: Don't show "Alternate text" under properties for images loaded directly → Don't show "Alternate text" under element properties for images loaded directly
Putting URL http://www.mozilla.org/images/mozilla-banner.gif as an example.

"Not applicable" would be adequate. Can Page Info just detect somehow that the
nsIImageDocument is being used and use "Not applicable" instead of the real ALT
string?
* Changing "images loaded directly" to "standalone images" which is what
developers use.
* Adding Page Info to the summary.
* Removing WONTFIX?? from status whiteboard
* Sending over to Page Info product. This can be fixed without modification to
ImageLib or nsIImageDocument, afaik using what I'll mention below. Modifying how
standalone images are shown when not found can be done in a seperate bug.
* Changing Severity to Trivial

Implementation Details:

http://lxr.mozilla.org/seamonkey/source/xpfe/communicator/resources/content/nsContextMenu.js#255

I think this is like what we'll need to do in Page Info and Element Properties:

var documentType = window._content.document.contentType;
if ( documentType.substr(0,6) == "image/" ) {

/* Change the ALT text to "Not applicable for standalone images." or something */

}
Assignee: pavlov → db48x
Severity: normal → trivial
Component: ImageLib → Page Info
Summary: Don't show "Alternate text" under element properties for images loaded directly → Don't show "Alternate text" under Element Properties and Page Info for standalone images
Whiteboard: WONTFIX?

Comment 23

15 years ago
The more proper fix should be the ALT text itself, NOT another hack to get
around that hack...
Aaron, then the other thing we could do is change the alt text to "Not
Applicable" and modify nsIImageDocument somehow when the picture didn't load,
but I am not sure if there is a way to know if the image didn't load properly
and then we get into a whole lot of work for something so small. The
communication between Imglib and nsIImageDocument is going to be improved
someday anyway, so this isn't really a huge hack, its just a 10 line temporary hack.

Comment 25

15 years ago
*** Bug 250239 has been marked as a duplicate of this bug. ***

Updated

15 years ago
Flags: blocking-aviary1.0RC1?
Flags: blocking-aviary1.0RC1? → blocking-aviary1.0RC1-
*** Bug 254584 has been marked as a duplicate of this bug. ***

Comment 27

15 years ago
*** Bug 260000 has been marked as a duplicate of this bug. ***

Comment 28

15 years ago
xref: bug bug 135902 - Alternate text and Type has wrong value
Product: Browser → Seamonkey
*** Bug 273632 has been marked as a duplicate of this bug. ***
*** Bug 289284 has been marked as a duplicate of this bug. ***
*** Bug 292432 has been marked as a duplicate of this bug. ***
*** Bug 296140 has been marked as a duplicate of this bug. ***

Comment 33

14 years ago
How about applying this special alt-text only if the picture doesn't load? 
Right now, if you drag a picture into a Thunderbird message window, then it is
possible to email it off perfectly fine, except the file name says "blah blah...
cannot be displayed, because it contains errors."  If there is no alt-text then
Thunderbird uses the file name wich looks much better (try the same thing using
Internet Explorer).
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20050825 SeaMonkey/1.0a]
(nightly) (W98SE)

Bug 292503 may have partially fixed the current bug:
*Drag and Drop from Windows Explorer: fixed.
*<about:logo>: still broken, but Reload fixes it...
*<http://www.mozilla.org/images/mozilla-banner.gif> (any http/https !!?):
remains broken.
*** Bug 341685 has been marked as a duplicate of this bug. ***
Created attachment 225815 [details] [diff] [review]
wip (diff -w)

Like so?
Created attachment 225816 [details]
Screenshot, Page Info
Created attachment 225817 [details]
Screenshot, Properties
I can't reproduce comment #34; I thought bug 292503 had fixed this.
(In reply to comment #39)
> I can't reproduce comment #34; I thought bug 292503 had fixed this.
> 

SeaMonkey and Firefox trunk on Linux still says:
Alternate Text: The image “http://www.mozilla.org/projects/minefield/minefield-icon.png” cannot be displayed, because it contains errors.

in Page Info and Properties for a standalone image.
(In reply to comment #40)
>SeaMonkey and Firefox trunk on Linux still says:
>Alternate Text: The image
>“http://www.mozilla.org/projects/minefield/minefield-icon.png” cannot be
>displayed, because it contains errors.
Weird. My linux trunk is OK. ajschult's linux trunk is OK. My Windows 1.8.1a is OK. But my Windows trunk is not ok...
(In reply to comment #41)
> Weird. My linux trunk is OK. ajschult's linux trunk is OK. My Windows 1.8.1a is
> OK. But my Windows trunk is not ok...

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1a3) Gecko/20060614 SeaMonkey/1.1a] (nightly) (W98SE)

Bug still there:
1. Open (in a new tab) the link from comment 34.
2. Image context menu > Properties.
(In reply to comment #36)
> Created an attachment (id=225815) [edit]
> wip (diff -w)
> 
> Like so?

(The idea seems right.)
Nit: I suggest to swap the test and the else order in the <pageInfo.js> file(s).
Created attachment 226137 [details] [diff] [review]
Patch
Attachment #225815 - Attachment is obsolete: true
Created attachment 226138 [details] [diff] [review]
Patch (diff -w)
Attachment #226138 - Flags: superreview?(neil)
Attachment #226138 - Flags: review?(gautheri)
Attachment #226138 - Flags: review?(gautheri) → review?(bryner)
Comment on attachment 226137 [details] [diff] [review]
Patch

Nits:
once you have review on th -w patch, I suggest to:

>Index: browser/base/content/metaData.js
>===================================================================
>RCS file: /cvsroot/mozilla/browser/base/content/metaData.js,v
>retrieving revision 1.9
>diff -p -u -1 -2 -r1.9 metaData.js
>--- browser/base/content/metaData.js	2 Jun 2006 04:28:48 -0000	1.9
>+++ browser/base/content/metaData.js	19 Jun 2006 12:37:03 -0000
>@@ -261,25 +261,26 @@ function checkForImage(elem, htmllocalna
>   if (!ismap) {
>-   if (imgType == "img" || imgType == "input") {
>+   if ((imgType == "img" || imgType == "input") &&
>+       !(elem.ownerDocument instanceof ImageDocument)) {
>      setAlt(img);
>    } else {
>      hideNode("image-alt");
>    }
>   }

move the if '{' to the next line,
and add a missing space to this if+else block.

>Index: xpfe/browser/resources/content/metadata.js
>===================================================================
>RCS file: /cvsroot/mozilla/xpfe/browser/resources/content/metadata.js,v
>retrieving revision 1.24
>diff -p -u -1 -2 -r1.24 metadata.js
>--- xpfe/browser/resources/content/metadata.js	18 Mar 2005 10:38:52 -0000	1.24
>+++ xpfe/browser/resources/content/metadata.js	19 Jun 2006 12:37:04 -0000
>@@ -210,25 +210,26 @@ function checkForImage(elem, htmllocalna
>     if (!ismap) {
>-       if (imgType == "img" || imgType == "input") {
>+       if ((imgType == "img" || imgType == "input") &&
>+           !(elem.ownerDocument instanceof ImageDocument)) {
>            setAlt(img);
>        } else {
>            hideNode("image-alt");
>        }
>     }

move the if '{' to the next line,
and remove the extra spaces from this if+else block, to get a two-space indentation only.
Created attachment 226181 [details] [diff] [review]
Patch rev. 3
Attachment #226137 - Attachment is obsolete: true
Attachment #226138 - Attachment is obsolete: true
Attachment #226138 - Flags: superreview?(neil)
Attachment #226138 - Flags: review?(bryner)
Created attachment 226182 [details] [diff] [review]
Patch rev. 3 (diff -w)
Attachment #226182 - Flags: superreview?(neil)
Attachment #226182 - Flags: review?(neil)
I'm sorry but I can't reproduce the "error" text bug on any of my builds today.
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1a3) Gecko/20060623 SeaMonkey/1.1a] (nightly) (W98SE)

Sometimes, I have to try more (2+) than one time,
but I can still get the _error_ text with both the <banner.gif> and <about:logo>.

The easiest seems to be to
1. Load the <banner.gif> in the current tab,
2. Context-click to open it in a new tab.

NB: In the cases when I don't get the "error" text, I get the "location" url.
Comment on attachment 226182 [details] [diff] [review]
Patch rev. 3 (diff -w)

When imagelib finds the image is already in the image cache it cancels the original load request with an error code (<biesi> it can't use a success code <biesi> necko won't let it). Unfortunately this prevents the image document from clearing the ALT text, so we need to tell the image document that NS_IMAGELIB_ERROR_LOAD_ABORTED is really a success code.
Attachment #226182 - Flags: superreview?(neil)
Attachment #226182 - Flags: superreview-
Attachment #226182 - Flags: review?(neil)
Created attachment 227029 [details] [diff] [review]
Patch rev. 4 (CHECKED IN: comment 57)

(In reply to comment #51)
> ... so we need to tell the image document that
> NS_IMAGELIB_ERROR_LOAD_ABORTED is really a success code.

Something like this? (The Alt. text with this patch is
"http://www.mozilla.org/projects/minefield/minefield-icon.png")

What about other errors, e.g. a corrupt image would still have Alt. text in
Properties/PageInfo for a image document with this patch.
As I see it, a standalone image doesn't conceptually have Alt. text because
that is an attribute only for markup documents, so Properties/PageInfo
should not list Alt. text at all. So, I prefer the last patch (rev. 3).
Attachment #227029 - Flags: superreview?(neil)
Attachment #227029 - Flags: review?(neil)
As I understand it,
'rev 3' would fix the initial report,
'rev 4' would be an extra (welcomed) fix.
Comment on attachment 227029 [details] [diff] [review]
Patch rev. 4 (CHECKED IN: comment 57)

I'm not a content peer; please find another reviewer.

Note: if you don't think successful image documents should have alt text then you could submit a patch to remove line 580.
Attachment #227029 - Flags: superreview?(neil) → superreview+
(In reply to comment #54)
>if you don't think successful image documents should have alt text then
>you could submit a patch to remove line 580.
Ah, it seems that the alt text is necessary for Select All/Copy to pick it up.

With this fix I no longer have an issue with the alt text in page info or properties, but if you are still unhappy then I suggest you continue the discussion with the module owner.
Comment on attachment 227029 [details] [diff] [review]
Patch rev. 4 (CHECKED IN: comment 57)

See comment 51, comment 52. This fixes the bogus Alt. text in
Properties/PageInfo. I think we should remove it from the UI completely
(for image docs) eventually, but that's a different patch.
Attachment #227029 - Flags: review?(neil) → review?(bzbarsky)
Attachment #227029 - Flags: review?(bzbarsky) → review+
Comment on attachment 227029 [details] [diff] [review]
Patch rev. 4 (CHECKED IN: comment 57)

Checked in at 2006-06-28 19:42 PDT.

Leaving the bug open to remove the Alt.text field in Properties/PageInfo.
Attachment #227029 - Attachment description: Patch rev. 4 → Patch rev. 4 (CHECKED IN: comment 57)
Looking forward to get this first patch into 1.8 branch...
(Assignee)

Comment 59

13 years ago
Comment on attachment 227029 [details] [diff] [review]
Patch rev. 4 (CHECKED IN: comment 57)

in that case, set the flag :)
Attachment #227029 - Flags: approval1.8.1?

Comment 60

13 years ago
Comment on attachment 227029 [details] [diff] [review]
Patch rev. 4 (CHECKED IN: comment 57)

Not comfortable making a chnage to nsImageDocument for FF2 beta1 to fix a minor UI issue.

It also seems to me that the correct fix would be to make ImageLib not report an error when it successfully sends data to its consumer.  The current patch appears to be a bandaid for bad ImageLib behavior :-(
Attachment #227029 - Flags: approval1.8.1? → approval1.8.1-
(In reply to comment #60)
>It also seems to me that the correct fix would be to make ImageLib not report
>an error when it successfully sends data to its consumer.  The current patch
>appears to be a bandaid for bad ImageLib behavior :-(
I'm no ImageLib cache expert but apparently it's unable to succesfully cancel the original channel, instead it is required to report an error.

Well you can't pass a success code to nsIRequest::cancel.

Comment 63

13 years ago
I don't understand.  Why doesn't ImageLib pass a dummy error code to Necko and then map that error code back to something like NS_OK before reporting it to its consumer?  Why must ImageLib's consumer know about this?  Do we have to go and hunt down every ImageLib consumer and fix them in the same way as nsImageDocument?
hm, that'd probably be a good idea

Comment 65

11 years ago
This WFM on Firefox 3 rc1. Right-click / Properties on an image has the alt text set as the image URL which I think is acceptable. Tools / Page Info doesn't show the alt text field at all.
(In reply to comment #65)

> Properties on an image has the alt
> text set as the image URL which I think is acceptable.

I think displaying the URL as alt text is wrong both because it's redundant and because it is not the content of an alt attribute of an img tag sent by a server.
It's a minor issue though.

> Tools / Page Info doesn't show the alt text field at all.

It was fixed (at least for Firefox) in Page Info in another bug.
QA Contact: tpreston

Comment 67

10 years ago
> Actual Results:  
> "Alternate text" says 'The image [image_name] cannot be displayed, because it
> contains errors.'
The original problem is fixed as it now shows the url of the image. As to whether this is correct or if this attribute should be hidden for images is gist for another bug.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → WORKSFORME

Comment 68

10 years ago
VERIFIED Windows NT 5.1; en-US; rv:1.9.1.5pre) Gecko/20091019
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.