Last Comment Bug 47475 - Need "Show(View) Image" / "Reload Image" on context menu
: Need "Show(View) Image" / "Reload Image" on context menu
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: Trunk
: All All
: P3 enhancement with 84 votes (vote)
: mozilla1.9alpha1
Assigned To: Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com]
: Terri Preston
Mentors:
: 35130 47792 101058 121786 140811 155384 161786 175712 176357 185518 209262 220910 232041 252594 255600 268063 283776 284006 285952 323416 (view as bug list)
Depends on: 75338 83774
Blocks: 164421 193867 715399 218142
  Show dependency treegraph
 
Reported: 2000-08-03 10:51 PDT by INOUE Seiichiro
Modified: 2014-07-20 13:00 PDT (History)
78 users (show)
shaver: blocking1.8.1-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proposed solution (6.68 KB, application/octet-stream)
2001-02-09 13:18 PST, Igor Nekrestyanov
no flags Details
Proposed solution (as ZIP file) (10.44 KB, application/zip)
2001-02-13 12:08 PST, Igor Nekrestyanov
no flags Details
nis@sparc.spb.su's patch (23.91 KB, patch)
2001-02-14 18:06 PST, timeless
no flags Details | Diff | Review
patch with style changes (42.42 KB, patch)
2001-02-15 08:07 PST, timeless
no flags Details | Diff | Review
Patch updated to work with main mozilla trunk (65.19 KB, patch)
2001-02-21 08:17 PST, Andrew Brygin
no flags Details | Diff | Review
SeaMonkey patch (6.05 KB, patch)
2005-09-21 11:14 PDT, Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com]
cbiesinger: review-
Details | Diff | Review
better patch (12.37 KB, patch)
2005-09-22 16:44 PDT, Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com]
cbiesinger: review+
bzbarsky: review+
neil: superreview+
neil: approval‑branch‑1.8.1-
Details | Diff | Review
unrotted (16.04 KB, patch)
2006-08-13 18:28 PDT, Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com]
no flags Details | Diff | Review
unrotted, addressing bz's review comments from before (16.14 KB, patch)
2006-08-13 20:05 PDT, Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com]
bzbarsky: review+
neil: superreview+
Details | Diff | Review

Description INOUE Seiichiro 2000-08-03 10:51:18 PDT
From Bugzilla Helper:
User-Agent: Mozilla/4.72 [en] (X11; I; Linux 2.2.17 i686; Nav)
BuildID:    2000073008

Netscape4x has a preference setting as follows,
"Automatically load images and other data types
(Otherwise, click the images button to load when needed)".
I like this feature, i.e. I can load images by clicking button on the images,
only when I want to see them.

Mozilla has a similar setting, "Do not load any images".
But it lacks the above feature.
When I choose this setting, I have no way to load images.


Reproducible: Always
Steps to Reproduce:
1.Choose "Do not load any images" setting.
2.Open an HTML file including some images.
3.Click button on an image.

Actual Results:  Nothing happens.
			

Expected Results:  I expect it to load the image.(only that image).

I think "Do not load any images" should be "Do not load any images
automatically".
Comment 1 sairuh (rarely reading bugmail) 2000-08-04 14:38:16 PDT
morse, this sounds like something in your area --is it?
Comment 2 Stephen P. Morse 2000-08-05 12:24:40 PDT
Certainly a nice feature and something I wasn't even aware of.  I now see that 
if you have image-loading disabled, then the right-click context menu has a 
show-image choice which will cause that individual image to get loaded.

Note that this is similar to (but not the same as) the view-image choice in the 
right-context menu.  However, in that case the image is brought up in a separate 
window.  Furthermore, it has a slight problem (see bug 34165).

Unfortunately this is a request for a new feature.  All new-feature work has 
been frozen until after the product finally ships.  So I have no choice but to 
"future" this bug, same as was done for 34165.  Now if someone out on the web 
wanted to implemented this, then we could get this incorporated prior to product 
ship.
Comment 3 Stephen P. Morse 2000-08-05 12:27:26 PDT
Corrected the wording on the summary line.  Previous wording was confusing. 
Comment 4 sairuh (rarely reading bugmail) 2000-08-07 11:23:08 PDT
this sounds almost identical to bug 34165. any reason why it isn't a dup? other
than the fact that in here all images are turned off...
Comment 5 Stephen P. Morse 2000-08-07 11:48:19 PDT
No, it's not a dup.  As I described above, show-image and view-image are two 
different features.  View image causes the image being pointed at to be 
displayed in a separate window.  Show image causes the image to appear in its 
designated place on the page.  This bug is about show image and bug 34165 is 
about view image.
Comment 6 sairuh (rarely reading bugmail) 2000-08-07 13:32:19 PDT
okay.
Comment 7 Rob Hodges 2000-08-10 00:02:49 PDT
"Now if someone out on the web wanted to implemented this, then we could get
this incorporated prior to product ship."

Alright, I am someone out on the web!  And this one is a showstopper for me.  
I've been looking at it lately but I'm not quite sure where this would fit into
the code.  Most of the other UI stuff seems to utilise the DOM but I'm not sure
that's enough here, and I'm not sure how deep I need to go to get this in.

The questions that come to mind are:
1a) Is it currently possible to load an image into its placeholder from
Javascript?
1b) If not, what is the appropriate code to look at (where do the js bindings
come from?)
2) Should I be looking at nsIPresContext::StartLoadImage() and if so, where do I
get the nsIPresContext object to invoke it?  Or am I totally off track?
3) Am I going to need to temporarily override the "don't load images" flag to
force that load?  (if so, how?)

I'd really appreciate some guidance as to the Right Way to go about this; any
documents/source you think I should read etc.  I'm having a hard time getting a
high-level view of the code, but I *really* want this feature.  (And a lot of
people are going to be spitting chips if it isn't in the first release.)  

(We also need a "Load all images" button on the toolbar when the "don't load
images" pref is enabled, although I guess that's up to the individual skin
authors.  Are the prefs exposed to xul?)
Comment 8 pnunn 2000-11-16 13:34:05 PST
*** Bug 48549 has been marked as a duplicate of this bug. ***
Comment 9 Paul Chen 2000-12-29 10:23:11 PST
nav triage team:

Nice to have, but not a beta1 stopper, marking nsbeta1-
Comment 10 Stephen P. Morse 2001-01-02 19:09:00 PST
Selective image blocking is done in the nsCookie.cpp file and so would get 
classified as a cookie bug.  Total image blocking is done in the image library.

This bug involves total image blocking.  So reassigning.
Comment 11 Igor Nekrestyanov 2001-02-09 13:18:45 PST
Created attachment 24923 [details]
Proposed solution
Comment 12 Igor Nekrestyanov 2001-02-09 13:19:53 PST
I just attached archive with proposed solution.

Overall idea:
  - Have separate service for ImagePermissions
     (unlike cookies they are volatile, not persistent)
  - When user explicitly request image displaying then
     add permissions for this image (or whole page) and
     trigger image(s) update
  - Use trick to trigger image update from javascript:
     clear and then restore location.href of corresponding image object
  - Move permission checks from image library to nsFrameImageLoader
     because we need to know to which document this image belongs
  - If image is prohibitted to be loaded for some reason
     (blocked of image disabled) then show it if it is already in cache
    (but do fetch it from the net)

See some additional comments in README.

Comment 13 Rich Burridge 2001-02-12 15:26:35 PST
tpreston, our engineers in Russia have a potential fix for this "bug".
Can you review it and give it r= for checking into the trunk please?
Thanks.
Comment 14 sairuh (rarely reading bugmail) 2001-02-12 20:01:09 PST
->pavlov, while pnunn's on sabbatical. rich, i cannot seem to view the
attachment --could it pls be reattached as a patch so that it can be reviewed?
thx!
Comment 15 Rich Burridge 2001-02-12 21:26:18 PST
Igor, could you please do as sairuh requests. Thanks.
Comment 16 Igor Nekrestyanov 2001-02-13 12:08:48 PST
Created attachment 25173 [details]
Proposed solution (as ZIP file)
Comment 17 Igor Nekrestyanov 2001-02-13 12:15:07 PST
I attached same set of patches again as zip archive
(first time it was .tar.gz) 
It is quite big - affects 14 files (3 are new) so i do not think that 
it is worth attaching each file separately.

To view it first save patch as zip file (say 47475.zip) and then unzip it.
Comment 18 sairuh (rarely reading bugmail) 2001-02-14 14:50:14 PST
pavlov [or someone else], could you pls review the attached patch? need to save
as zip, then unzip 'em...
Comment 19 timeless 2001-02-14 16:47:58 PST
cvs server: warning: 
mozilla/xpfe/browser/resources/content/Attic/nsContextMenu.js is not (any longe
r) pertinent

I have a long list of comments, i'll attach a cvs diff -N -u -w
Comment 20 timeless 2001-02-14 18:06:17 PST
Created attachment 25320 [details] [diff] [review]
nis@sparc.spb.su's patch
Comment 21 timeless 2001-02-15 06:28:50 PST
line 33/34 of nsFrameImageLoader.cpp.patch
>   nsCOMPtr<nsIURI> url;
>   url = doc->GetDocumentURL();
instead
>   nsCOMPtr<nsIURI> url(doc->GetDocumentURL());
[recently scc seems to have changed his mind about this one, but we've been 
encouraging this style for a while].

>   (void)url->GetSpec(&spec);
why (void)?

>     NS_PRECONDITION(nsnull != mGroup, "null ImageGroup ptr");
>     if (nsnull == mGroup) {
>       return NS_ERROR_NULL_POINTER;
>     }
>     NS_PRECONDITION(nsnull == mImageRequest, "double init of ImageRequest");
>     if (nsnull != mImageRequest) {
>       return NS_ERROR_ALREADY_INITIALIZED;
>     }

instead you could write:
>     NS_PRECONDITION(mGroup, "null ImageGroup ptr");
>     if (!mGroup) {
>       return NS_ERROR_NULL_POINTER;
>     }
>     NS_PRECONDITION(!mImageRequest, "double init of ImageRequest");
>     if (mImageRequest) {
>       return NS_ERROR_ALREADY_INITIALIZED;
>     }
-

>           switch(permission) {
>             case nsIImagePermission::LOAD_WHOLE_GROUP :
>               mGroup->SetImgLoadAttributes(grouploading_attribs |
nsImageLoadFlags_kExplicitlyRequested);
>             case nsIImagePermission::LOAD_ALLOWED :
>               break;
>             case nsIImagePermission::LOAD_FORBIDDEN :
>               attribs |= nsImageLoadFlags_kOnlyFromCache;
>               mImageLoadStatus = NS_IMAGE_LOAD_STATUS_NOT_PERMITTED;
>               break;
>             default:
>               //by default just load image
>               break;
>           }
>         } else {
>         return rv;
>         }

is ::LOAD_ALLOWED similar to default?
>           switch(permission) {
>             case nsIImagePermission::LOAD_FORBIDDEN :
>               attribs |= nsImageLoadFlags_kOnlyFromCache;
>               mImageLoadStatus = NS_IMAGE_LOAD_STATUS_NOT_PERMITTED;
>               break;
>             case nsIImagePermission::LOAD_WHOLE_GROUP :
>               mGroup->SetImgLoadAttributes(grouploading_attribs |
nsImageLoadFlags_kExplicitlyRequested);
>             case nsIImagePermission::LOAD_ALLOWED :
>             default:
>               //by default just load image
>               break;
>           }
>         } else {
>           return rv;
>         }

--
>     char* cp = mURL.ToNewCString();
i think we're moving to a different syntax...
[mURL.get() which is a pointer you don't need to delete]

...navigator.xul.patch
>                   accesskey=""
since you're defining accesskeys as "" in the dtd, you should reference them.
accesskey="&showImageCmd.accesskey;" and 
accesskey="&showAllImagesCmd.accesskey;" respectively.  So if
someone later wants to set an accesskey they don't need to change the xul.

nsContextMenu.js.patch
>       //cludge to force image to be redisplayed
i think mozilla.org is using XXX to represent things that are bad/broken
so:
>       //XXX cludge to force image to be redisplayed

*** Patching a file in /Attic/ is bad. Attic means it was deleted. ***

nsImagePermission.cpp
please include a modeline and a license (mpl1.1 is suggeested)

function
PRBool nsImagePermission::permitLoad
has 4spaces for indentation, the rest of the file seems to use 2spaces.  It's 
your call [i like 2 spaces,
but consistency is more important]

// static data initialization
COOKIE_BehaviorEnum nsImagePermission::mImageBehavior = COOKIE_Accept;
nsVoidArray* mDocPermission = nsnull;
nsVoidArray* mImgPermission = nsnull;

Statics are sometimes risky, i'll defer to someone else who will say they're 
ok.

if.cpp.patch
while rewriting please switch from NULL to nsnull

Please don't license files under NPL unless you are a netscape employee.
Comment 22 timeless 2001-02-15 08:07:24 PST
Created attachment 25339 [details] [diff] [review]
patch with style changes
Comment 23 timeless 2001-02-15 08:09:54 PST
if people find my changes to Igor's patch acceptable, we can include them. 

Igor: if you don't want to do the extra cleanup, I can file another bug for it.
Comment 24 Igor Nekrestyanov 2001-02-15 08:28:45 PST
timeless: Thank you for usefull comments. I have no idea on 
recent style/syntax changes and tried to follow the way things were 
done in mozilla sources.
I am working on updated version of patch. 
Anyway i want to remove some dead stuff from nsCookie.cpp 
(that is now in nsImagePermission) (so i will make some cleanup)

Soory about Attic - this is because i am mostly working with not with 
main branch and i did not realise that nsContextMenu.js was moved.
Comment 25 Stuart Parmenter 2001-02-15 13:03:19 PST
thanks for the patch, however most of this code is being rewritten, so checking
this in now will only cause some new problems.  The javascript/xul stuff will be
useful, but I want to hold off checking this in until later.
Comment 26 Igor Nekrestyanov 2001-02-15 15:16:57 PST
Proposed solution was based not on the main branch
(and this was my mistake, sorry). I will try to update 
it to new changes and resubmit.

BTW, both timeless's patches will not work. 
they both do not include changes to nsContextMenu.js (which was moved 
and therefore my patch went to /Attic/ :( ). 
Also patch with style changes does not contain any newly added files
as well as change to nsIFrameImageLoader.h
 
Comment 27 Stuart Parmenter 2001-02-15 15:35:12 PST
the new code isn't on tip yet.  in about 2 weeks it should all be there and at
that point we should get this working.
Comment 28 timeless 2001-02-15 16:08:54 PST
arg. i thought i did cvs diff -N -u, sorry. i can rediff, but at this point it 
doesn't matter.  i'll send igor the diff -N -u 's because i did make changes to 
his files.
Comment 29 Andrew Brygin 2001-02-21 08:17:08 PST
Created attachment 25803 [details] [diff] [review]
Patch updated to work with main mozilla trunk
Comment 30 Andrew Brygin 2001-02-21 08:18:08 PST
I just attached newer version of proposed solution                              
that was updated to work with current state of main mozilla trunk.              
We tried to follow style changes proposed by timeless.
Comment 31 Gregory Labzovsky 2001-05-18 08:53:39 PDT
pavlov:  Is anybody going to review and check in the patch atteched by Andrew
Brygin?
Comment 32 Stuart Parmenter 2001-05-20 14:28:25 PDT
unfortunatly, a large part of the patch is no longer valid since the new 
imagelib landed.  some of it may still be however I think there are some better 
ways to do it, such as using nsIContentPolicy, and just resetting the src to 
the same thing.

however, I havn't thought about it too much, so don't hold me to what I just 
said :-)
Comment 33 stefan livens 2001-07-27 04:37:56 PDT
I almost filed a new bug for this one, but I stumbled on this one. 
How is the situation on this one ?
I definitely want this one in the browser asap, because it's much needed. 
why ? browse the apple site, and others, they use the akamai server to deliver
content. I have 'only load images from originating server' to block ads, but I
can't load the images on the apple site, so the site is basically useless. no
'unblocking images from this site' doesn't cut it.
So back to main question : when ?
Comment 34 Dimitrios 2002-01-25 07:28:33 PST
*** Bug 121786 has been marked as a duplicate of this bug. ***
Comment 35 Manko 2002-01-25 07:43:42 PST
It's important that Gecko unlike NN 4.x allows show image without reloading the
whole document.

More, when image is loaded, context menu item "Show image" could be changed to
"Reload image" - this is very useful for dynamic CGI-scripted images such as
diagrams, counters or informers.
Comment 36 Matthias Versen [:Matti] 2002-04-28 19:00:54 PDT
*** Bug 140811 has been marked as a duplicate of this bug. ***
Comment 37 Jesse Ruderman 2002-07-02 11:53:22 PDT
We might want to rename "show image" to "load image".  "Load image" is more
similar to "reload image" and less likely to be confused with "view image".

(After writing this comment, I noticed that mpt made the same suggestion in
http://www.mozilla.org/projects/ui/menus/shortcut/.)
Comment 38 Jesse Ruderman 2002-07-02 11:56:25 PDT
*** Bug 155384 has been marked as a duplicate of this bug. ***
Comment 39 Jesse Ruderman 2002-07-02 11:56:35 PDT
*** Bug 35130 has been marked as a duplicate of this bug. ***
Comment 40 Daniel Wang 2002-08-09 00:44:57 PDT
*** Bug 161786 has been marked as a duplicate of this bug. ***
Comment 41 Boris Zbarsky [:bz] (Out June 25-July 6) 2002-09-12 00:08:54 PDT
*** Bug 101058 has been marked as a duplicate of this bug. ***
Comment 42 Jesse Ruderman 2002-10-21 02:02:45 PDT
*** Bug 175712 has been marked as a duplicate of this bug. ***
Comment 43 Jesse Ruderman 2002-10-21 02:12:53 PDT
*** Bug 47792 has been marked as a duplicate of this bug. ***
Comment 44 wayne harper 2002-10-23 14:41:06 PDT
this is a must have for those of us still using dial-up. with it you can keep
images turned off & only view the ones you want.
Comment 45 Daniel Wang 2002-11-03 16:35:36 PST
*** Bug 176357 has been marked as a duplicate of this bug. ***
Comment 46 Felix Miata 2002-12-15 15:15:23 PST
Anyone else think bug 185518 should be duped to this?
Comment 47 Kai Lahmann (is there, where MNG is) 2002-12-15 16:12:21 PST
*** Bug 185518 has been marked as a duplicate of this bug. ***
Comment 48 Eugene Savitsky 2003-01-21 17:16:51 PST
Nominating. Seen thoused times questions about this feature in discussions on
the net in all kind of forums (russian, estonian, international). Very needed
function for notebook/PDA internet users thrue GPRS (an GSM internet access).
Comment 49 Boris Zbarsky [:bz] (Out June 25-July 6) 2003-03-13 10:39:27 PST
Once bug 83774 lands this may not be that hard to do.... (just have to be
careful for the security issues).
Comment 50 Joachim Noreiko 2003-04-24 15:18:04 PDT
This context-menu option should be available at all times, irrespective of the
"Do not load any images" preferences setting.

here's why: on a modem, a page with many images may take a long time. But a user
can often tell which images will interest them more - from reading ALT text, or
from the surrounding text. 
It should be possible to right-click the image placeholder, select "Load this
image" to move it to the front of the "image-getting queue".
Comment 51 Samir Gehani 2003-04-25 15:09:58 PDT
adt: nsbeta1-
Comment 52 rstupp 2003-05-28 06:19:21 PDT
Hi!

Just an issue, why people may want to disable image loading in Mail&News:
Spam Mails often embed image locations to track, whether an email-address is
valid and used (read by someone).

So disabling automatic image loading in mail & news is a nice feature.

Greetings,
Robert
Comment 53 Christian :Biesinger (don't email me, ping me on IRC) 2003-06-13 09:01:59 PDT
*** Bug 209262 has been marked as a duplicate of this bug. ***
Comment 54 Christian :Biesinger (don't email me, ping me on IRC) 2003-06-13 09:04:57 PDT
my suggestion from bug 209262:
Well, if you ask me, we should use the current "view image" option for this:
o) If the image is not currently loaded, load the image and display it in place
o) If it is, show only the image (=current behaviour)
Comment 55 Manko 2003-06-15 23:50:48 PDT
Christian, it seems to me your suggestion isn't handly. If image is already
loaded it will be nice use "Load/Reload Image" context menu item for reload
image. See comment 35 and comment 37.

Showing only selected image (without HTML document) is, I guess, less essential
feature. If you consider it to be necessary we could add another item in context
menu named e.g. "Show Only This Image" by analogy with iframe context menu.

More, with same analogy, for reducing context menu we could hide all image menu
items in a submenu, e.g.:
This Image >
  ---------------------
  Load/Reload Image
  ---------------------
  Block Images from this Server
  Block Images and Cookies from this Server
  ---------------------
  Show Only This Image
  Open Image in New Window
  Open Image in New Tab
  ---------------------
  Copy Image Location
  Save Image as...
  Send Image...
  Set As Wallpaper
  ---------------------
  View Image Info
  Open Long Description
  Open Long Description in New Window
  Open Long Description in New Tab
Comment 56 Christian :Biesinger (don't email me, ping me on IRC) 2003-06-16 08:52:06 PDT
Eh, I don't think that we need an entire submenu for image-related commands
Comment 57 Manko 2003-06-19 00:54:04 PDT
Maybe :) but Reload command for loaded images seems to be more handly that View
command.
Comment 58 JonStipe 2003-07-17 17:30:13 PDT
My ISP has had a problem lately where, when downloading a web page with lots of
images, the download of some images might get corrupted sometimes, causing them
to appear all messed up (it's a dial-up connection). Since it only seems to
happen when lots of images are being downloaded at once, a normal reload will
just reproduce the problem. Currently, the only solution is to right-click a
damaged image, select "View image", reload just that image, and go back. Having
a "reload image" option would help a lot for this.
Comment 59 Brett Taylor 2003-09-02 19:39:35 PDT
Isn't this fixed?  Suggest to mark as RESOLVED.
Comment 60 Christian :Biesinger (don't email me, ping me on IRC) 2003-09-03 04:36:07 PDT
>Isn't this fixed?

no.
Comment 61 Jo Hermans 2003-10-01 08:36:43 PDT
*** Bug 220910 has been marked as a duplicate of this bug. ***
Comment 62 Stefek Zaba 2003-10-03 06:16:13 PDT
Pretty please: this is a usability and privacy feature which is clunky to
work around. In the absence of a single-click-action whose effect is "show me
the page populated with images, temporarily overriding my
image-blocking-preferences", Joe Luser - me, in this case - either wanders off
to Edit->Prefs->Priv&Sec->Images
and selcts the "take me now, big boy" option - "Accept All Images", *intending*
(but probably, nay, provably) forgetting to reset it afterwards; or, far worse,
fires up A.N.Other browser (Exploder, mayhap), with loss of loyalty/mindshare to
Mozilla. It's one neat idea from Netscape 4.x which is, IMNSHO, worth carrying
across. But what do I know...

It's obviously non-trivial, or the request wouldn't still be in the Bugzilla DB
three years after first being raised; and it's in the Wishlist FAQ, so I'm not
the only Netscape refugee hankering after it. All I can offer in further support
is "think of the undying gratitude of the userbase, and the growth in karma"... 

Thanks, Stefek
Comment 63 James Carr 2003-10-03 06:45:30 PDT
an important bug being fixed? dream on
Comment 64 Christian :Biesinger (don't email me, ping me on IRC) 2003-10-03 07:26:13 PDT
can you people STOP ADDING USELESS COMMENTS, they make it even less likely that
a bug will be fixed (because it's harder to spot the relevant information).
thank you.
Comment 65 James Carr 2003-10-03 08:05:58 PDT
clearly additional comments are interupting all the technical discussions one
how this bug is going to be fixed. whats that you say? nothing has been done for
how long? oh well never mind. hail the comment hitler.
Comment 66 Christian :Biesinger (don't email me, ping me on IRC) 2003-10-03 09:19:54 PDT
well after that comment, my inclination to fix this bug is no longer existing.
Comment 67 flii (cj) 2003-10-03 21:59:08 PDT
re: comment #66
you mean someone's actually working on it?  awesome!  i'm not sure how much help
i would be since i use firebird instead of mozilla, but when you come out with a
patch i will sure test it out if it would do any good.

i know this would be a particularly nice feature when you block ads by blocking
key terms in usercontent, such as "banner", and then a site you actually want to
see pictures on has named their main logo "topbanner" or something similar. 
i've been waiting for mozilla to get it implemented so it will trickle down to
firebird, and it's heartening to know someone is taking it seriously.  best of luck!
Comment 68 Manko 2003-10-06 04:56:53 PDT
Please, do not argue about necessity of fixing this bug here. Your activity
affects only negatively.

Could anybody see attachment 258038 [details] [diff] [review] and adapt it to current Mozilla trunk?

And, please, add DO NOT SPAM to Status Whiteboard.
Comment 69 MAX 2003-10-12 07:24:08 PDT
Browsing without images still do not work.
So option "do not load any images" - senseless.
Turn off images and go to
http://www.tomshardware.com
 We see wrong page layout so we can not read headlines. IE-> OK.
http://www.pornstaraction.com/
 We do not see image borders and alttext so we can not right-click on image
 placeholder to load and view DESIRED image. IE -> OK.
So browsing without images DO NOT WORK! Is this bug???
PLEASE do browsing without images:
1 Turn off images (may be from customized toolbar)
2 Load page with:
  CORRECT layout
  Image borders (it MUST exist for right-clicking, as i understand if IE can 
  not determine image size it make default minimal square ~100x100mm -> so 
  borders always exist for right-click)
  Image alt-text (it MUST exist, for desision view or not)
3 Right-click menu with option view image. And load ONLY DESIRED image WITHOUT
  open new or reload page.

Comment 70 Alfonso Martinez 2004-01-24 08:53:59 PST
*** Bug 232041 has been marked as a duplicate of this bug. ***
Comment 71 R.K.Aa. 2004-03-12 22:35:44 PST
*** Bug 237334 has been marked as a duplicate of this bug. ***
Comment 72 Jo Hermans 2004-07-22 04:18:37 PDT
*** Bug 252594 has been marked as a duplicate of this bug. ***
Comment 73 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2004-08-14 07:07:35 PDT
*** Bug 255600 has been marked as a duplicate of this bug. ***
Comment 74 Dimitrios 2004-09-05 14:01:18 PDT
Users interested in this bug may look at ShowImage extension at
http://showimage.mozdev.org/. It is made for Firefox. It didn't work very well
for me (maybe it needs updating) but it seems a good approach (no further
cluttering of the context menu for users with fast connections).
Comment 75 Justin 2004-10-14 00:46:15 PDT
Just a small note: This functionality exists in IE 6. Set IE to not load images.
Right click on an image placeholder, 5th option down "Show Picture". And it
loads the image without reloading the page.
Comment 76 Christian :Biesinger (don't email me, ping me on IRC) 2004-11-06 06:01:30 PST
*** Bug 268063 has been marked as a duplicate of this bug. ***
Comment 77 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-02-27 16:25:51 PST
*** Bug 283776 has been marked as a duplicate of this bug. ***
Comment 78 :Gavin Sharp [email: gavin@gavinsharp.com] 2005-03-08 15:34:53 PST
*** Bug 284006 has been marked as a duplicate of this bug. ***
Comment 79 Dan Fischbach 2005-03-08 15:50:30 PST
Similar bug?:
https://bugzilla.mozilla.org/show_bug.cgi?id=218142
Comment 80 Dan Fischbach 2005-03-08 15:53:09 PST
Ack, I'm an idiot...Bugzilla needs an edit button/link.
Comment 81 R.K.Aa. 2005-03-13 05:32:48 PST
*** Bug 285952 has been marked as a duplicate of this bug. ***
Comment 82 Mike Connor [:mconnor] 2005-07-11 01:03:26 PDT
The firefox bug depending on this was already minused.
Comment 83 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2005-09-21 11:14:51 PDT
Created attachment 196930 [details] [diff] [review]
SeaMonkey patch
Comment 84 Christian :Biesinger (don't email me, ping me on IRC) 2005-09-21 15:58:40 PDT
Comment on attachment 196930 [details] [diff] [review]
SeaMonkey patch

	 // View Image depends on whether an image was clicked on.
	 this.showItem( "context-viewimage", this.onImage &&
!this.onStandaloneImage);
-

why remove that line?

I'm not really happy about showing this for images that are not broken. I'd
prefer it if reload for those images actually used a VALIDATE_ALWAYS load flag,
which this patch doesn't. r=biesi on this patch if you make it only appear for
broken images (this code has a check for those already, iirc)
Comment 85 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2005-09-22 16:44:29 PDT
Created attachment 197106 [details] [diff] [review]
better patch
Comment 86 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-09-23 08:05:08 PDT
Comment on attachment 197106 [details] [diff] [review]
better patch

Just looking at the core changes here....

>Index: mozilla/content/base/public/nsIImageLoadingContent.idl

>+  /**
>+   * forceReload reloads the last image that was attempted to be loaded
>+   * (as indicated by currentURI), using VALIDATE_ALWAYS.

How about "forceReload forces reloading of the image pointed to by currentURI"?
 VALIDATE_ALWAYS is, imo, an implementation detail that consumers of this API
should not have to know about....

>+   * @throws NS_ERROR_NOT_AVAILABLE if this isn't an image or no load was
>+   * attempted yet

How about "throws NS_ERROR_NOT_AVAILABLE if there is no current URI to reload"?

With that, looks good to me.
Comment 87 :Gavin Sharp [email: gavin@gavinsharp.com] 2005-09-23 08:10:23 PDT
Comment on attachment 197106 [details] [diff] [review]
better patch

>Index: mozilla/xpfe/communicator/resources/content/nsContextMenu.js

>+        this.showItem( "context-reloadimage", this.onImage);

>+    // Reload image
>+    reloadImage : function () {
>+        urlSecurityCheck( this.imageURL, document );
>+        if (this.target instanceof Componenets.interfaces.nsIImageLoadingContent)
>+          this.target.forceReload();

If you're here, this.onImage should be true, and this.onImage already checks
instanceof nsIImageLoadingContent (and currentURI != null).

http://lxr.mozilla.org/seamonkey/source/xpfe/communicator/resources/content/nsC
ontextMenu.js#287
Comment 88 :Gavin Sharp [email: gavin@gavinsharp.com] 2005-09-23 08:29:14 PDT
(I also failed to notice the "Componenets" typo :)
Comment 89 Christian :Biesinger (don't email me, ping me on IRC) 2005-09-26 17:10:39 PDT
Comment on attachment 197106 [details] [diff] [review]
better patch

despite gavin's observations, I'd rather keep the check... not only does this
way make it clearer that forceReload is a function of nsIImageLoadingContent,
but the code is also separated in time and space from the one that sets
onImage.

very nice. r=biesi
Comment 90 neil@parkwaycc.co.uk 2005-10-10 16:33:35 PDT
Comment on attachment 197106 [details] [diff] [review]
better patch

>+<!ENTITY reloadImageCmd.label         "Reload Image">
>+<!ENTITY reloadImageCmd.accesskey     "R">
This currently conflicts with "Save Link Target". However should
https://bugzilla.mozilla.org/show_bug.cgi?id=146825#c40 get implemented it will
then conflict with "Set As Wallpaper" but I might possibly be able to change
that to use "p".
Comment 91 neil@parkwaycc.co.uk 2005-10-11 03:42:29 PDT
Comment on attachment 197106 [details] [diff] [review]
better patch

It took me some time to work out what this code did (basically by testing it on
a hit counter), as it appears to be of no use on blocked images.
Comment 92 neil@parkwaycc.co.uk 2005-11-20 04:56:47 PST
Comment on attachment 197106 [details] [diff] [review]
better patch

Well, I don't really see the utility of this particular patch, but please get the context menu access key conflicts fixed before checking this in.
Comment 93 Artem S. Tashkinov 2005-11-29 08:25:47 PST
Is there any chance to see this bug resolved in MoFo/SeaMonkey Trunk versions?
Comment 94 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2005-12-30 16:20:47 PST
Comment on attachment 197106 [details] [diff] [review]
better patch

Requesting approval on 1.8 branch for SM1.1 due to (minor) core change.
Comment 95 Lloyd Wood 2005-12-31 00:16:46 PST
fixing status whiteboard typo's.
Comment 96 :Gavin Sharp [email: gavin@gavinsharp.com] 2005-12-31 07:04:11 PST
It wasn't a typo :)
Comment 97 Mike Connor [:mconnor] 2006-01-05 18:19:14 PST
tossing to 1.8.1 list, I don't think we want this particular patch for Firefox, and this is contains an API change so it shouldn't be approved as-is for 1.8.1.
Comment 98 Manko 2006-01-10 05:21:07 PST
Question about patch: will Shift + "Reload Image" do forced reloading (similar as Shift+Reload for whole page)?
Comment 99 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2006-01-10 12:39:34 PST
(In reply to comment #98)
> Question about patch: will Shift + "Reload Image" do forced reloading (similar
> as Shift+Reload for whole page)?
> 

I wrote another version that supports shift just like the reload button, but for some reason it doesn't work at all.
Comment 100 Ria Klaassen (not reading all bugmail) 2006-01-14 08:11:11 PST
*** Bug 323416 has been marked as a duplicate of this bug. ***
Comment 101 neil@parkwaycc.co.uk 2006-01-30 17:08:53 PST
Comment on attachment 197106 [details] [diff] [review]
better patch

My understanding is that we're not allowed to change nsIImageLoadingContent.idl
Comment 102 Mike Shaver (:shaver -- probably not reading bugmail closely) 2006-04-18 10:23:25 PDT
blocking-: we wouldn't hold for this RFE.  (Neil could certainly approve a patch for the branch for SM, and mconnor or someone for FF.)
Comment 103 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2006-06-01 14:53:13 PDT
Neil, what key should I use?
Comment 104 neil@parkwaycc.co.uk 2006-06-01 16:07:52 PDT
(In reply to comment #103)
>Neil, what key should I use?
No idea offhand; the current set of keys are all wrong anyway, see bug 146825.
Comment 105 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2006-08-13 18:28:51 PDT
Created attachment 233526 [details] [diff] [review]
unrotted
Comment 106 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2006-08-13 18:31:15 PDT
Neil, for the XPFE side, let's say I used "G" as the access key.
Comment 107 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2006-08-13 20:05:27 PDT
Created attachment 233530 [details] [diff] [review]
unrotted, addressing bz's review comments from before
Comment 108 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-08-13 20:12:28 PDT
Comment on attachment 233530 [details] [diff] [review]
unrotted, addressing bz's review comments from before

>Index: content/base/src/nsImageLoadingContent.cpp
>+  if (!currentURI)
>+    return NS_ERROR_NOT_AVAILABLE;

Curlies around the body, please.

With that the content parts look good.  I didn't look at the XUL/JS changes...
Comment 109 neil@parkwaycc.co.uk 2006-08-14 05:29:40 PDT
Comment on attachment 233530 [details] [diff] [review]
unrotted, addressing bz's review comments from before

only xul/js change was typo fix right?
Comment 110 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2006-08-14 05:43:55 PDT
(In reply to comment #109)
> (From update of attachment 233530 [details] [diff] [review] [edit])
> only xul/js change was typo fix right?
> 

Yes, and I now patch both xpfe/ and suite/.
Comment 111 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2006-09-07 15:54:04 PDT
Checked in on trunk (SeaMonkey 1.5).  Not going on 1.8 branch as discussed before.
Marking fixed because the core component of this bug is fixed.  UI for Firefox is in bug 218142.
Comment 112 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2006-09-29 18:56:43 PDT
Resolving based on comment 111 (apparently I forgot to click the button?)
Comment 113 Nickolay_Ponomarev 2006-09-30 04:34:56 PDT
This didn't actually implement 'Show image', does it? (forceReload doesn't work with images turned off.)
Comment 114 Worcester12345 2006-10-02 09:37:20 PDT
(In reply to comment #0)
> From Bugzilla Helper:
> User-Agent: Mozilla/4.72 [en] (X11; I; Linux 2.2.17 i686; Nav)
> BuildID:    2000073008
> 
> Netscape4x has a preference setting as follows,
> "Automatically load images and other data types
> (Otherwise, click the images button to load when needed)".
> I like this feature, i.e. I can load images by clicking button on the images,
> only when I want to see them.
> 
> Mozilla has a similar setting, "Do not load any images".
> But it lacks the above feature.
> When I choose this setting, I have no way to load images.
> 
> 
> Reproducible: Always
> Steps to Reproduce:
> 1.Choose "Do not load any images" setting.
> 2.Open an HTML file including some images.
> 3.Click button on an image.
> 
> Actual Results:  Nothing happens.
> 			
> 
> Expected Results:  I expect it to load the image.(only that image).
> 
> I think "Do not load any images" should be "Do not load any images
> automatically".

Where do we enable this feature in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061001 SeaMonkey/1.5a?
Comment 115 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2006-10-02 16:13:38 PDT
(In reply to comment #114)
> Where do we enable this feature in Mozilla/5.0 (Windows; U; Windows NT 5.1;
> en-US; rv:1.9a1) Gecko/20061001 SeaMonkey/1.5a?

The scope of the bug I fixed was: if images are not blocked, when right-clicking an image or broken image, it should be possible to force a reload.  The justification for this behavior is dailup users who stop a page load early and only want to load specific images (or broadband users dealing wth slow servers).  There is no way to "enable" the feature - just right-click a [non-css-applied] image.
Comment 116 Nebojsa 2006-10-05 14:39:45 PDT
I think that if I hit right-click on the broken image it shouldn't be reloaded in new window or tab. I need to see it on it's place. Not to mention the sites with so many little pics close to one another as a part of website's design. Developers sould reconsider putting this option as an add-on (plug-in) and who wants it...
If something like that exist, please reply here.
Comment 117 Christian :Biesinger (don't email me, ping me on IRC) 2006-10-05 16:50:32 PDT
That's now part of SeaMonkey. File a new bug for Firefox I guess.
Comment 118 Manko 2007-04-25 01:16:28 PDT
This isn't fixed in SM 1.1.1, menu item doesn't appear.
Comment 119 Philip Chee 2014-07-14 07:10:40 PDT
*** Bug 715399 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.