281492 - Alt text not displayed when images disabled

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[PikeUK]

When images are disabled (i.e. Uncheck Tools->Options->Web Features->Load Images
in Firefox) the alternative text is not displayed in place of the image.

Steps to reproduce:
1. Disable images.
2. Visit www.google.com

The google logo should be replaced with the word "Google", in current nightlies
it is blank.

Using:
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b) Gecko/20050206 Firefox/1.0+

Regression window:
Works: 2004-12-15-08 (Firefox and Seamonkey)
Fails: 2004-12-16-07 (   "     "      "    )

Perhaps Bug 240070 is the cause?

p.s. Not sure of correct component, sorry if this is wrong.

Comment 1

[Asa Dotzler [:asa]]

mvl, can you take a look at this?

Comment 2

[Michiel van Leeuwen (email: mvl+moz@)]

relevant code from the old nsImgManager:
216     if (!shouldLoad)
217       *aDecision = nsIContentPolicy::REJECT_REQUEST;

and from teh new nsContentBlocker:
182   if (!shouldLoad)
183     if (fromPrefs)
184       *aDecision = nsIContentPolicy::REJECT_TYPE;
185     else
186       *aDecision = nsIContentPolicy::REJECT_SERVER;

I guess the alt text only gets shown when returning REJECT_REQUEST. The problem
is that nsIContentPolicy is a bit amiguous imo. the type and the server are both
 details of the request. So should it return _SERVER or _REQUEST?

The bug can be fixed by returning _request from nsContentBlocker, instead of
_type or _server. But is that the right thing to do?

Comment 3

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: patch v1

It turns out gecko doesn't really look at the return value of shoudload, but
reads the pref directly. And i forgot to update that code.
The real fix will be to make it look at the different return values, but for
now this patch should work.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 173768 [details] [diff] [review]
patch v1

sr=bzbarsky

Comment 5

[[e-mail]]

Comment on attachment 173768 [details] [diff] [review]
patch v1

r=me, but we really should have a proper fix for b2 i think. this image code
bites ;)

Comment 6

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: updated patch

I missed some user of the old pref in the first pref.
With this new pref, there are still a few in profile migrator code, but those
can wait. They will just set the pref, and that pref will be migrated
automaticly.

Comment 7

[[e-mail]]

Comment on attachment 173787 [details] [diff] [review]
updated patch

r=me for b1, but i sure would like to know why prescontext cares about this
pref too :/

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 173768 [details] [diff] [review]
patch v1

Wait. What about fixing kIconLoadPrefs ?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 173787 [details] [diff] [review]
updated patch

sr=bzbarsky, but note tha the prescontext code is unused, as far as I know.

Comment 10

[Michiel van Leeuwen (email: mvl+moz@)]

patch checked in. the right fix will be in bug 202906