Closed Bug 109090 Opened 23 years ago Closed 23 years ago

[FIX] Blocked image should not show Broken Icon

Categories

(Core :: Layout, defect, P4)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.7

People

(Reporter: william, Assigned: attinasi_layout)

References

Details

(Keywords: regression, Whiteboard: [bae:20011127])

Attachments

(2 files)

PATCH to prevent the ALT text and broken image icon from displaying for blocked images
1.79 KB, patch
alexsavulov
: review+
kinmoz
: superreview+
Details | Diff | Splinter Review
before: sections shown, able to navigate. after: not
44.61 KB, image/png
Details 
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:0.9.5+)
Gecko/20011108
BuildID:    2001110803

With this build, I find that blocked images show up as broken, and shows the
"alt=" text.  If I block an image from a server, I prefer the image space stay
blank.  This was the previous behaviour.

I didn't find another bug on this issue, if there is one that had discussion
saying it should have this behaviour, dup this, and I will join that discussion.

Reproducible: Always
Steps to Reproduce:
1.Load a page with a blocked image.
2.
3.

Actual Results:  Broken image icon.

Expected Results:  Blank spot.
I vote for a wontfix.
You see the broken image (because the image is blocked) and you can click on it 
if the image contains a link.
(And there is somehwere a bug in bugzilla [Duped] that a broken image should 
show the broken image icon)
But the image is not broken, its blocked.  I made a move to ask it to be
blocked.  Perhaps there should be a preferences option asking what Mozilla
should do with blocked images, either show as broken or leave a blank spot.  At
the very least, it should have a different icon.  I think knowing whether there
is a problem with the site vs. blocking is important.

The big win for me with blocking images, beyond the privacy issue of 3rd parties
registering a hit, was that I have some control over being inundated with ads. 
Now I get the ads again, in text form.
Alternatively, the space occupied by the blocked add
could collapse, removing the empty space and any link
associated with it.
Changing QA owner
Status: UNCONFIRMED → NEW
Ever confirmed: true
QA Contact: petersen → tpreston
Bug 41924 discusses this issue. The proposal there is to display a different
iamge for blocked images, and possibly offer the user the option to hide that
icon and/or collapse the image into its ALT text (as an inline).
Status: NEW → ASSIGNED
Priority: -- → P4
Target Milestone: --- → mozilla1.0
See the newsgroup posting for the pref to get the old behavior:

news://news.mozilla.org:119/3BE981BB.3060607@netscape.com
Severity: trivial → enhancement
*** Bug 109702 has been marked as a duplicate of this bug. ***
See also 48705, which wants an alternate icon.
The preference change listed in the bugfix does NOT affect this bug.  I still
see the "broken" link for blocked images, no matter what the preference is set
to... and I'm using user_pref, not the incorrect "pref" listed in the newsgroup
article.  

Please treat this bug as a regression and back out the buggy patches that were
made for 0.9.5 (so we have no box, no text) which was MUCH better than the
ridiculous behaviour we have now with the "broken" icon.

The correct solution will be to shrink the box to zero size (as if the image was
never there) and make this selectable through the GUI.
Target Milestone: mozilla1.0 → mozilla1.2
This is sad.  Why in the world is this moved to Mozilla *1.2* when it used to
work up until 0.9.5?

Just back out the regression!
no test case necessary
Whiteboard: [bae:20011127]
Comment #5: No. Bug 41924 discusses *broken*, not *blocked* images.

Comment #6: the preference has no effect on this behavior (as already commented
by Allan)

Once more: this is a regression, not enhancement. This worked till recently.
Changing severity to normal.
Severity: enhancement → normal
Keywords: mozilla0.9.7, regression
Yes, blocked images should show a different Icon, and in an ideal world, the
user should be able to set a preference for whether or not to show the icon
(possibly the alt-text too?). Problem is, I am only one person, and I have to
prioritize my bugs - this is less important than many other bugs on my plate.
Please, feel free to dig in and try and make some progress on this. I'd be happy
to shepard along any reasonable patches that fix this. Or, if somebody wants to
figure out how to tell if an image has been blocked vs. broken that would help a
lot too. 

Alan:
>...and I'm using user_pref, not the incorrect "pref" listed in the newsgroup
>article.
pref works in all.js, which is where I indicated you could set it. user_pref
works in the user pref files, which is apparently where you chose to set it.

>This is sad.  Why in the world is this moved to Mozilla *1.2* when it used to
>work up until 0.9.5?
Yes, it is sad. It is all about priorities and limited resources. It is equally
sad that in the face of overwhelming bug lists and limited time, the community
chooses to bellow and complain instead of lending a hand.

>Just back out the regression!
No can do. It is interesting that the pref does not change the behavior for
blocked images though - I'll look into that.
Keywords: mozilla0.9.7
> pref works in all.js, which is where I indicated you could set it. user_pref
> works in the user pref files, which is apparently where you chose to set it.

The all.js file is for default prefs. It is no place for user prefs as it
becomes overwritten with every Mozilla reinstallation. That is exactly why we
have prefs.js and user.js for user preferences. 

Therefore there is no excuse for not registering the pref properly, when set
from prefs.js or user.js.

I checked with about:config that the pref (browser.display.force_inline_alttext)
is registered when set from user.js. Meaning the user.js pref overrides all.js
setting, as expected.

But still I see no difference between "true" and "false" settings.
What would be involved in fixing this? I can take a look at it but my worry
(from discussing it on #mozillazine) is that it involves fairly deep
layers-of-abstraction problems (i.e. how does the renderer know that a missing
image was blocked in some other module and not just a broken link). Also my
impression from the beginning has been that the image blocking stuff was widely
thought to be misdesigned for similar reasons. I don't think I'm really
qualified to go monkeying around with such global stuff, but if someone can
point me in the direction of a sufficiently local fix for the narrow current
issue I could give it a shot.
How about a fix were, at the point of rendering the broken image and alt-text,
it looks up the domain in the blocked images database.  If present, skips that
logic.

Bad, and slow, I know, but a work around until it cna be redesigned...
I looked at this briefly and I think it is pretty easy. In the ImageFrame there
is a method CanLoadImage that checks the content load policy to see if the image
is effectively blocked. If it is blocked, the loading routine simply fails
making it impossible to distinguish between a broken and blocked image (either
will cause the same failure in the loading routine RealLoadImage). If we set a
flag in the CanLoadImage indicating that the image was actually blocked, as
opposed to invalid, then we can check that flag and do the old behavior (or
whatever other behavior we want) for the blocked case. I'll try to get to this
in my spare time, unless somebody beats me to it ;)
Here is a patch to detect blocked images and prevent the display of the broken
image icon and the alt text. Seems to give the same behavior as we got before
my changes to ALT text display. Check it out, please.
Patch attached
Assignee: attinasi → attinasi_layout
Status: ASSIGNED → NEW
Keywords: patch
Summary: Blocked image should not show Broken Icon → [FIX] Blocked image should not show Broken Icon
Target Milestone: mozilla1.2 → mozilla0.9.7
Status: NEW → ASSIGNED
Based on comments in other bugs and elsewhere, it might be desirable to
wrap a pref arround this behavior (default to current unpatched behavior).
mythdraug@pobox.com, could you please be more specific? My understanding is that
_nobody_ likes the current behavior for *blocked* images. My patch does not
impact loading or broken images, just blocked ones. I have not seens anyone in
support of the broken image icon for blocked images, but I may have missed it in
the storm of comments complaining about it ;)
Feel free to ignore my previous comment.  :)

It was mostly based on a discussion that took 
place on #mozillazine earlier today. Someone 
(perhaps playing devil's advocate) suggested that
the current behavior was technically correct, but
conceded that it may not be expected by end users.

I thought I remembered seeing similar comments in
the many other bugs related to this one, but didn't
search for them again.
I've tested the patch on WindowsME. Works fine. The behavior is really the same
as with for example 0.9.2 (I had it installed and checked for comparison). No
noticeable side effects.

Shouldn't this get the r= and sr= now?
Regarding whether this should be wrapped in a preference, I also got the
impression from the #mozillazine conversation, and from reading some of the
other bugs leading up to this, that some people strongly feel that correct
behavior is to do *something* to flag blocked images. I don't feel that
way, but I can restate the case.

One scenario that was mentioned is a cybercafe, where you sit down and a screen
comes up missing a bunch of images and you have no idea what's wrong. In this
kind of case it might be good to have a "blocked image" icon, distinct from the
"broken link" icon, that, perhaps together with a tooltip or something, could
give you some clue how to fix the problem. Similarly, if I accidentally block
too many images, it will be easier for me to fix the problem if there is some
visual indication on the page of where I can rightclick to fix things up.

Some people also seemed to think that correct behavior standards-wise is ALWAYS
to display ALT text for any image that isn't displayed for whatever reason.
(This idea also comes across in the proposal in bug 41924.) This seems a
particularly weak point to me -- if you've deliberately blocked an image, surely
you don't want to be seeing a text version of it instead. If the standards don't
recognize this, I suspect it's because they fail to contemplate that people
might be deliberately blocking images they don't want to see.

If these feelings are actually widespread, that would be a reason to control
this behavior with a preference -- maybe a reasonable comprimise default would
be to show a blocked image icon but not any alt text.
I believe that the proposal linked in bug 41924 covers both using an alternate
icon for blocked images, and the pref to make the icon visible or not. I'd
prefer to keep that enhancement in that bug, and just deal with the alleged
'regression' here. Sound OK?

BTW: I don't really want to go down the slippery slope of trying to guess why
people block images - there are certainly many reasons for doing this. It might
be better to give users options about what gets displayed for blocked images
(eg. 'Show Alt Text for Blocked Images' and 'Allow Blocked Images To Take Up
Space'). Some users will block images because they ahve low bandwidth, and they
will wnat to preserve the layout and see the alt text. Others may well want the
blocked image to 'disappear' entirely, supressing the alt text and the layout
box. I'd like to make both classes of users happy, if possible.

Anybody want to r= this bug? I'll seek sr once we have reviews. Thanks for
trying it out Jacek, can you r=?
Comment on attachment 59571 [details] [diff] [review]
PATCH to prevent the ALT text and broken image icon from displaying for blocked images

r= alexsavulov

To display another icon ok, but isn't the ALT text just a cosmetic detail? If
we keep giving importance to such minor issues the next wish will be to set a
user defined text like: "Good block dude!" or so :-) Marc, I agree with your
comment #13. Anyone who cares about such things should dig into the source and
make a patch ;-)
Attachment #59571 - Flags: review+
Comment on attachment 59571 [details] [diff] [review]
PATCH to prevent the ALT text and broken image icon from displaying for blocked images

sr=kin@netscape.com
Attachment #59571 - Flags: superreview+
Checked into trunk.

Checking in nsImageFrame.h;
/cvsroot/mozilla/layout/html/base/src/nsImageFrame.h,v  <--  nsImageFrame.h
new revision: 1.42; previous revision: 1.41
done
Checking in nsImageFrame.cpp;
/cvsroot/mozilla/layout/html/base/src/nsImageFrame.cpp,v  <--  nsImageFrame.cpp
new revision: 1.217; previous revision: 1.216
done
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
I can no longer navigate manu sites now. Many sites have stupid nav bar eyecandy
like img="section1.gif" alt="section 1" that I block to speed page loads. Now
there is no longer a button to click, or text to even show me what's missing.

Attaching screenshot comparing an old build to todays at www.theregister.co.uk.
The Register is a news site, and I have no need from images from there ever.
Now, I have no choice but to turn them all back on so I can see the sections.
Jeremy, you can still navigate it. Try holding your mouse over the area where
you know the navigation bar is. The links there are still live, although it is
admittedly hard to know which is which.

This case really sounds like a "don't do it then" kind of thing though.
I simply put this back the way it had been for a long tim - sorry if you liked
the other behavior better. Once we have a 'blocked image' icon, I think you will
be happy again. See bug 41924
Verified fixed linux build 2001120308, win XP build 2001120303 and Mac OS X
2001120308
Status: RESOLVED → VERIFIED
*** Bug 113709 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: