Closed Bug 180620 Opened 22 years ago Closed 21 years ago

Images don't display their alt text when images are disabled (have to restart browser to make pref take effect)

Categories

(Core :: Layout: Images, Video, and HTML Frames, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla1.7alpha

People

(Reporter: ian, Assigned: dbaron)

References

()

Details

(Keywords: testcase, Whiteboard: [patch])

Attachments

(2 files, 1 obsolete file)

Our spec for this is http://www.hixie.ch/specs/alttext

If an image cannot or will not be shown, then:

  1. For images that are blocked, we should do nothing. (As if the element
     had 'display:none'.)

  2. a. For images that have not been downloaded but are about to be and
        that have 'height' and 'width' attributes set, in 'Always Load
        Images' mode, and
     b. For all images with 'height' and 'width' attributes when the
        document is in legacy (or 'quirks') mode:

     We should use a placeholder.

  3. For ALL other cases (blocked images, images not downloaded in 'Don't
     Load Images' mode, broken images, missing images, unrecognised
     images, and images with no 'height' and 'width' attributes), we should
     simply insert the alt text directly into the page instead of the image.

Note that this should also work when an image is changed from one category to
another, e.g. in quirks mode if an image that is broken with no height and width
attributes (category 3) has height and width attributes added, it should gain a
placeholder frame (switch to category 2).


At the moment we do the wrong this when images are disabled (we always seem to
fall into category 2).
Blocks: 90169
Priority: -- → P3
Target Milestone: --- → Future
Using build 2002101612 on Win98, display of images when they are disabled
seems downright inconsistent:

- Sometimes the alt text is displayed as plain text in place of the image.
- Sometimes an empty frame is displayed.
- Sometimes a frame is displayed with the alt text inside the frame. (Best
  solution that I've found.)
- Sometimes nothing is displayed whatsoever, leaving the user completely
  unaware there's supposed to be an image there at all--which is very
  confusing on some pages.  For example:

http://www.xml.com/pub/a/2002/10/02/metaschema.html?page=2

Scroll down a little less than halfway, you should find the following text at
the end of a paragraph:

"Here's another piece of the WXS metamodel, showing how type extension is
implemented:"

Under this should be an image...but if images are turned off in Mozilla, the
image is completely nonexistent.  There is no indication that an image is
supposed to be there, leaving the user scratching his head wondering what the
web page author is referring to.  Viewing source shows that there is alt text
on the image, but the alt text is not displayed.  In short, there's no
indication that this page even contains diagrams if viewed with Mozilla with
images turned off.

By the way in the alttext spec listed above:  What's the difference between
"images that are blocked" in item #1 and "blocked images" in item #3?
(That was a typo, fixed in the actual spec linked above. Blocked images should
follow item #1 not #3.)
*** Bug 194132 has been marked as a duplicate of this bug. ***
If blocked images are speced to be ignored, then I respectfully suggest that
that spec should be changed. 

Where images are blocked for either security or bandwidth reasons, it's
generally good to know that there's supposed to be something there -- so that
you can download it if you find it interesting enough to go against your general
policy.

It's also necessary in a case where a blocked image serves a hotlink.
Ah.. found it ...  in  http://www.hixie.ch/specs/alttext :

   For ALL other cases (<b>images not downloaded in 'Don't Load Images'
   mode,</b> broken images, missing images, unrecognised images, and
   images with no 'height' and 'width' attributes):

      In CSS terms, make the IMG or INPUT be a non-replaced element.

      If there is any alternative text then insert an inline box
      containing an image icon inside the IMG or INPUT element.

      If there is any alternative text, then insert it after the image
      icon.
samuel: where does that paragraph mention blocked images? What have blocked
images have to do with this bug, anyway?
OS: Windows 2000 → All
Hardware: PC → All
I think I misread the comments. It seemed like there was an argument that
turning off images was the same as blocking them -- which implied that there
should be no display whatsoever of the image.

This was, It now seems an improper reading.  I guess I didn't get enough sleep
last night. About the only argument in charge of my reading is that sometimes
with images turned of, the effect looks about the same as one would (properly)
expect for blocked (e.g. per-server) images -- dead air.

It looks to me like the proper result with images turned off would be an 'image
not displayed' icon and the alt text.
Hixie, which parts of this are still broken in current builds?
Unsure. Needs testing.
Attached file Testcase
I ran the situations listed in the bug description thru this testcase using
build 2003041709 (editing the testcase to get quirks mode, break images, etc.) 
Everything worked by the spec in standard and quirks mode except when images
were disabled via the Mozilla pref.  In that instance, in both rendering modes:


There was no placeholder frame but space was left in the layout for the images
(behavior like visibility:hidden as opposed to display:none).

The ALT text was not displayed.
Ok, thank you.  This may get fixed with bug 202506.
Depends on: 202506
I wanted to point out a few more specific details of this that I
have noticed...

- If images are disabled in the prefs, broken images still display their
  placeholder boxes and etc. (according to the spec).  However images
  on that same page which aren't blocked in any way--other than because
  image display is disabled--simply don't appear (as described above).
  Meaning, it's not an all-or-nothing thing, when images are disabled.
- Local pages seem to be processed completely differently.  A local copy
  of a page will (a) display images even if images are turned off (and
  even if those images are remote images), and (b) ALWAYS use inline text
  for ALT text replacement in the event that an image is broken.  For this
  test, I took a page that I created which is in Quirks mode, made an image
  link to a non-existant (e.g. broken) image, and viewed it from my local
  drive.  The ALT text was displayed inline; no image frame boxes.
  Page->Info still says "Quirks mode."  When I upload this same exact page
  to a remote server and view it from there, I get the image frame
  boxes.

Incidentally Mozilla is currently unusable when image loading is disabled,
which is why I consider this fairly critical...
Checking today with build 2003042204, the fix for bug 202506 has not fixed the
problem in comment 10.
Bill, are you restarting after changing the "do not load images" pref?  Dynamic
changes to that pref are known to be broken (though I'm not sure there is a bug
on it).

There also seems to be a problem with the "accept images from originating server
only" setting.....
No, I hadn't restarted.  Wasn't aware that there were issues with that.

After toggling the pref and restarting, the ALT text did appear correctly. (Does
someone want to write a bug about the pref issue now?)

That resolves all the main issues originally filed in the bug report, though I
don't know how the items in comment 12 should be addressed before considering
the bug resolved.
Depends on: 202904
Depends on: 202906
Bugs filed on the remaining issues that I can reproduce...  Keeping this open as
a tracker.
> - If images are disabled in the prefs, broken images still display their
>   placeholder boxes and etc. (according to the spec).

If images are disabled, they can't be broken as well. Anyway, according to the
spec (http://www.hixie.ch/specs/alttext), broken and disabled images are treated
identically.

bz: A few days before you want to start fixing alt text bugs in earnest (i.e. a
few days before you plan to start actually implementing this spec for real) let
me know and I'll test us to work out exactly what needs doing. The bugs in this
area seem to vary regularly and I can't keep track of the changes.
*** Bug 198629 has been marked as a duplicate of this bug. ***
from our feedback alias
<quote>
I notice that Netscape 6.2 and later do not treat "ALT" tags the same way
Netscap 4.x or IE 5 and up to.  For Example, the Columbia Lighthouse of the
Blind web site (http://www.clb.org/) makes extensive use of these tags --
perhaps they are usable with JAWS or WindowEyes, but if you turn off graphics,
there is no alt tag! 
</quote>

Aaron, what do you think about this with regard to accessibilty ?
Bob, I checked with some of the folks in W3C's Web Accessibility Initiative
(WAI) on this. They're pretty much on the side of leaving it the way that it is now.

It's a pretty complicated issue and I can see both points of view. My personal
opinion is that it would be nice if the W3C would be more clear on issues like
this, and I am on the side of showing alt as tooltips. However, my opinion
doesn't really matter at this point because it's become a kind of religious fight. 

We should leave it alone because the WAI won't officially support the idea of
non-existent title attributes repaired by ALT text.
This bug is about alt text incorrectly _not_ being shown when images are
_disabled_, and has nothing at all to do with tooltips.
My bad. For the most part things work better than in 6.2. There are still some
bugs blocking this. It looks like the layout folks have it under control.
The feedback actually came from a Federal agency with respect to Voter
assistance programs. I have asked them to provide their opinion on why this
should be fixed.

Does not showing ALT text when images are blocked effect accessibility for
disabled users or affect a web developer who wishes to test/view his site with
images disabled?
When images are _blocked_, we shouldn't show alt text.

When images are disabled, unavailable, or broken, we should show the alt text,
and it is indeed an accessibility problem if we do not.
->Image: Layout
Assignee: other → jdunn
Component: Layout → Image: Layout
Keywords: testcase
QA Contact: ian → tpreston
When images are blocked, users should be able to specify whether to show
alternate text or not -- logically speaking, they may have blocked the image 
(a) because it was annoying, in which case the text should NOT be rendered, or 
(b) because it was slow and/or unreadable, in which case the alternate text
SHOULD be rendered.

Because we do not know which of these options is the case, the default
assumption should be (b), especially when it is the case that ALL images have
been disabled.

For the case of images not originating from the same server as the document (and
especially for explicitly blocked images), it seems reasonable to assume (a),
but then there needs to be a user-specifiable option regarding the treatment of
alternate text in that case, for example enable a "show alternate text" checkbox
next to the "load images" checkbox.
*** Bug 218418 has been marked as a duplicate of this bug. ***
Mozilla 1.4 and 1.5b-0908, under Windows 2000; turning off images using 
Preferences | Privacy&Security | Images | Do not accept any images.

I am seeing 'alt' text displayed for the test case, for the URL provided in 
comment #1, or for my own test pages.

On the other hand, if I block the images (using the per-site permissions 
mechanism) no alt text is shown.

Therefore, this bug WFM; can I have an Amen?

No icons accompany the alt text, so bug 180622 is still an issue.
It does seem we are doing well here, but someone needs to go through the points
in the initial comment on this bug and check that they are all correctly addressed.
I'm not sure if this would be a separate bug, but it's the same concept.
When images are put in a document via the OBJECT tag (as required in
XHTML2), the "failover" text does not display when images are
disabled.

Example: <object type="image/png" data="test.png">Testing 1 2 3</object>

With images disabled, I would expect the text "Testing 1 2 3" should
be shown instead of the image test.png.
Ok, I just tested this a bit and the main bug is simply that the image pref is
not dynamic.

This bug basically useless for modem users who realise that disabling images
massively speeds up their browsing experience:

   with mozilla: go into prefs. disable images. restart browser. surf web. find 
   a page for which you want images. go into prefs. enable images. view page. go
   into prefs. disable images. oops, have to restart again. surf web.

   with opera: press g. surf web. find a page for which you want images. press 
   g. view page. press g. surf web.
Severity: normal → major
QA Contact: tpreston → core.layout.images
Summary: Images don't display their alt text when images are disabled → Images don't display their alt text when images are disabled (have to restart browser to make pref take effect)
The problem here is that nsImageFrame::IconLoad::GetPrefs caches prefs and
doesn't register observers, and nsImageFrame::HandleLoadError checks the pref it
cached.
That's what it looks like, anyway, although the iconload object is a
sort-of-a-singleton, but it depends on the current pres context and is supposed
to come and go.  This is ugly code.
Assignee: jdunn → dbaron
Target Milestone: Future → mozilla1.7alpha
Attached patch patch (obsolete) — Splinter Review
I couldn't resist a bit of cleanup.
Attachment #140026 - Flags: superreview?(bz-vacation)
Attachment #140026 - Flags: review?(bz-vacation)
Status: NEW → ASSIGNED
Whiteboard: [patch]
Comment on attachment 140026 [details] [diff] [review]
patch

>   LoadIcons(aPresContext);

I changed this to:

   if (!gIconLoad)
     LoadIcons(aPresContext);

but I don't think it's worth a new patch.
Is/will it be possible tp have the disable-images option on a per-window/tab
basis rather than globally?
Opera has this feature and it's great - only have images enabled for the pages
you want/need them in. (technically opera has 3 modes - images, manually
loaded/cached images or no-images. The second is similar to NS4.x with images
disabled - something I still use).
Hey, I figured, while you're working on it.. :-)

No.  And please don't clutter this bug anymore with discussion of that issue.
Attached patch patchSplinter Review
This fixes a few DEBUG-only compilation errors as well.
Attachment #140026 - Attachment is obsolete: true
Attachment #140100 - Flags: superreview?(bz-vacation)
Attachment #140100 - Flags: review?(bz-vacation)
Attachment #140026 - Flags: superreview?(bz-vacation)
Attachment #140026 - Flags: review?(bz-vacation)
Comment on attachment 140100 [details] [diff] [review]
patch

>+const char* const kIconLoadPrefs[] = {

drepper explained that
static const char kIconLoadPrefs[40][] = {
is better since it ends up in the rodata section and there's size savings from
pointers and relocations, as well as smaller codesize (one load rather than
two)
Comment on attachment 140100 [details] [diff] [review]
patch

>Index: nsImageFrame.h

>+  PRBool mDestroyed;

There's actually no need for this; more below.

>Index: nsImageFrame.cpp

>@@ -258,109 +258,93 @@ nsImageFrame::Destroy(nsIPresContext* aP
>     NS_REINTERPRET_CAST(nsImageListener*, mListener.get())->SetFrame(nsnull);

Note this.  After this point, we will get no imageloader notifications...

>+  nsIPresContext *presContext = GetPresContext();

Any reason not to use aPresContext here?  Or at least move this up to above the
LoadIcons call and use presContext there too?

> nsImageFrame::OnStartContainer(imgIRequest *aRequest, imgIContainer *aImage)
> {
>   if (!aImage) return NS_ERROR_INVALID_ARG;
>-  NS_ENSURE_TRUE(mPresContext, NS_ERROR_UNEXPECTED);  // why are we bothering?
>+  NS_ENSURE_TRUE(!mDestroyed, NS_ERROR_UNEXPECTED);  // why are we bothering?

And per above, this will never get called if we have been destroyed.  So
mDestroyed can just go.

>+  aImage->SetAnimationMode(GetPresContext()->ImageAnimationMode());

>+    nsIPresShell *presShell = GetPresContext()->GetPresShell();

Maybe save the GetPresContext() result that first time?

> nsImageFrame::OnDataAvailable(imgIRequest *aRequest,

>+      Invalidate(GetPresContext(), *aRect, PR_FALSE);

>+    Invalidate(GetPresContext(), r, PR_FALSE);

Again.

> nsImageFrame::OnStopDecode(imgIRequest *aRequest,

>+  nsIPresShell *presShell = GetPresContext()->GetPresShell();

>+          Invalidate(GetPresContext(), r, PR_FALSE);

Again.

> PRBool nsImageFrame::HandleIconLoads(imgIRequest* aRequest, PRBool aLoaded)
>+      if (aLoaded)
>+        if (++gIconLoad->mIconsLoaded == 2)
>+          gIconLoad->mLoadObserver = nsnull;

Why not:

if (aLoaded && (++gIconLoad->mIconsLoaded == 2))

?

>+nsImageFrame::IconLoad::IconLoad(imgIDecoderObserver *aObserver)
>+       pref < pref_end; ++pref)

pref != pref_end may be more readable... Either way.

r+sr=bzbarsky with that.
Attachment #140100 - Flags: superreview?(bzbarsky)
Attachment #140100 - Flags: superreview+
Attachment #140100 - Flags: review?(bzbarsky)
Attachment #140100 - Flags: review+
Fix checked in to trunk, 2004-02-03 12:30 -0800.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
*** Bug 202904 has been marked as a duplicate of this bug. ***
*** Bug 236506 has been marked as a duplicate of this bug. ***
Product: Core → Core Graveyard
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: