33724 - "Block Image" is misnamed

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect, P3)

Description

[Tom Neff]

The Block Image feature is nice, but it is not at all clear from the context of
using it that after right-clicking on any image at a site and selecting the
deceptively phrased "Block Image," in fact *all* images from that site will be
blocked *indefinitely* unless and until you hunt down the place to re-enable it.

You should move "Block Image" from the right-click context menu up into the
main View menu, and rename it "Block all images from this site".  When the user
selects it, you might want to pop up a dialog box reminding them that images
from the site will be blocked until further notice, and that to unblock they
need to go Edit->Preferences->Cookies and Images->View Blocked Images.

Comment 1

[Asa Dotzler [:asa]]

-> UI Design Feedback.  Warnings sound like a good idea to me.

Comment 2

[Matthew Mastracci]

What about a popup submenu on the menu called "Image Blocking ->", and have
"Block this site" or "Unblock this site" as checked menu items?  I like having
the convenience of right-clicking to block an image.  As well, you only really
want to block images that usually aren't part of a page, like a banner, which
means it *has* to stay in the popup menu.  

Comment 3

[Jesse Ruderman]

The dialog should mention which site is being blocked, and let the user change 
it (from ads5.bannerads.com to *.bannerads.com, for example).  It might also 
include an option to only block images from *.bannerads.com that show up when 
you're not actually browsing *.bannerads.com

Comment 4

[Matthew Mastracci]

Agreed...  I think that IE5 does something similar with trust zones.  Perhaps
this should be looked at for post-beta for a program-wide system.  I believe
someone was working on something like this for Javascript security - it could
probably get mixed in with cookies and images in the future.  This would
*definately* be an asset to the browser.  I'm sure I asked about this on one of
the mozilla groups - I'll see if I can't find the URL to the project page for
Javascript security by domain.

Comment 5

[Brendan Donohoe]

Reassigning to German for comment.

Comment 6

[Tom Neff]

Before this drifts in too many directions... Since my initial report I read 
comments on two different features.

[1] Various schemes to go ahead and do whole-site image blocking in a more 
sensible way.
[2] Actual blocking of a specific image or class of images, rather than 
blocking from a given site.

My objection is that the present UI confuses these two, giving you what LOOKS 
like a per-image blocking command when you right-click on an image, when 
actually it is a sitewide block.

Comment 7

[Eli Goldberg]

QA Assigning non-confidential New/Assigned User Interface: Design Feedback bugs 
to Matthew Thomas ([e-mail]).

Matthew Thomas is now the QA owner for the User Interface: Design Feedback 
component. (Bugs that involve UI issues in the Netscape-branded Mozilla browser 
should continue be QA assigned to [e-mail].)

Comment 8

[Matthew T (active 1999-2002)]

* `Block all images from this site' would be both slightly inaccurate and too
  long for a menu item. I'd suggest `Block images from server'.

* Moving this item into the View menu wouldn't really work, unless you required
  the image to be selected first (since a single page could contain images from
  multiple servers). In that case, using a context menu would still help prevent
  the problem of more than one image being accidentally selected.

* Submenus for context menus are never a good idea.

* Warnings for potentially destructive actions are almost always a good idea.

* It would be nice if this context menu item was a toggle which could be turned
  on and off in the context menu. However, that would be prevented if bug 23691
  was implemented (I've commented there accordingly).

Comment 9

[Jesse Ruderman]

If clicking the option brings up a warning dialog, the dialog should include:

- the hostname that's about to be blocked
- an indication as to whether that hostname is the same as the hostname of the 
webpage
- the number of images on the page from the hostname about to be blocked

Comment 10

[shuang (gone)]

should we mark this WON'TFIX? 
Change Severity to "Minor" and set to M18 so we'll have a solution after PR2.

Comment 11

[Matthew T (active 1999-2002)]

Suggestion: the context menu item say `Block Image ...', and bring up a dialog 
which allows you to
* turn blocking of images from the same domain (at any level) on/off
* turn blocking of images of the same size on/off.

Provided that blocked images still had a placeholder icon, I think that this 
suggestion would solve all the problems here:
* too easy to block images by mistake (fixed by using dialog);
* hard to undo image blocking (fixed by selecting item from context menu for
  placeholder graphic, and turning off blocking);
* more information needed about effect of blocking (fixed by using dialog).

Comment 12

[german]

I believe the context menu is no longer in the builds as reported. Should we 
close this bug then, or do you want to keep it open to keep the idea alive for a 
context menu item alive?

Comment 13

[Matthew T (active 1999-2002)]

I'm gonna mark this as a dup of bug 33576, where I specced a dialog for the image 
blocking context menu item. The dialog would solve both of the problems reported 
in this bug (lack of clarity for what the item does, and ability to undo the 
blocking).


*** This bug has been marked as a duplicate of 33576 ***

Comment 14

[Stephen P. Morse]

This is not a dup of 33576.  That bug refers to problems caused when the 
pref is set to reject images from foreign sites.  This bug deals with the 
right-click menu feature of "block image" and what does it mean.

It doesn't quite mean block all images from this site (where by "this" I assume 
you mean the current site).  Instead it means block all images from the site 
that sent the image at which you are pointing.  So if someone has a good 
name-substitution for "block image", I'll gladly make that change and close out 
this bug report.  Very simple fix -- no need to pop up any new dialogs.

Comment 15

[Stephen P. Morse]

Removing "dup" indication and reopening

Comment 16

[Stephen P. Morse]

*** Bug 47776 has been marked as a duplicate of this bug. ***

Comment 17

[Tom Neff]

>> So if someone has a good 
name-substitution for "block image", I'll gladly make that change and close out 
this bug report.  Very simple fix -- no need to pop up any new dialogs. <<

I haven't checked the code closely, but would it be possible to make the 
context menu text dynamically include the site name you're actually going to be 
blocking from? e.g. instead of "Block this image," say

 * Block all images from adimages.moneyhole.com

or whatever the site name you'd be about to add to the list would be?  Might as 
well be informative!

Comment 18

[Matthew T (active 1999-2002)]

If the menu item's behavior is dependent on a pref (that is, it's non-obviously 
modal), there's no way you can explain it solely in the menu item text without 
popping up a dialog anyway. IMO.

Comment 19

[Blake Ross]

This should definitely not be futured, this is a very important UI issue.  I 
agree that the wording needs to be changed, but I think it's also weird that 
this menu item only appears in the context menu if you've right clicked on an 
image, even though the item affects the entire site.  Why must you right click 
on a specific image to block images from the entire site?

Comment 20

[hangas]

Marking P3 bugs as nsbeta3-

Comment 21

[german]

Image managment will not be included in NS6. This is a mozilla only feature for
now. Forwarding to mozilla UI person (Ben) to figure out what to do with it.

Comment 22

[Mats Palmgren (inactive)]

*** Bug 55559 has been marked as a duplicate of this bug. ***

Comment 23

[Matthew T (active 1999-2002)]

*** Bug 69149 has been marked as a duplicate of this bug. ***

Comment 24

[Matthew T (active 1999-2002)]

stephend: Here's one of those DTD changes of which you are so fond. :-) Change 
`Block Image' to `Block Images from this Server'.

Comment 25

[Jesse Ruderman]

Instead of "Block Images from this Server", how about "Block Images from 
ad.doubleclick.net"?  Or would that be too long?  (And how about a dialog that 
gives the user a chance to cancel or choose to block *.doubleclick.net instead?)

Comment 26

[timeless]

yes please, although that will mean a properties entry, but stephend should be 
able to do that before this week ends :-)

Comment 27

[[e-mail] (gone - use [e-mail] instead)]

I know how to do mpt's request, but no others.

Comment 28

[[e-mail] (gone - use [e-mail] instead)]

Punting back to Ben.

Comment 29

[Matthew T (active 1999-2002)]

Undoing the blocking, and more fine-grained blocking using a dialog, is bug 
33576. I repeat, bug 33576.

This bug is just a stop-gap measure to help alleviate the confusion caused by 
`Block Image' blocking more than just that image. As such, it's just a string 
change, which stephend (or hwaara) could do.

As for `Block Images from x24.folderol.images.foo.com', that would be just too 
long. Even `Block Images from this Server' is pretty long, but it's ok for 
temporary purposes until bug 33576 is fixed.

Comment 30

[Blake Ross]

Stephen, as Matthew said, this is just an intermediary bug to change the 
wording (until the underlying problem is fixed).  That said, can you take this 
and make the change?

Comment 31

[Blake Ross]

Meant to give this to Stephen.

Comment 32

[[e-mail] (gone - use [e-mail] instead)]

Blake: http://lxr.mozilla.org/seamonkey/search?string=blockImageCmd.label shows 
that this doesn't exist (except in a cookie overlay).  Am I supposed to add 
this, again?  

Comment 33

[[e-mail] (gone - use [e-mail] instead)]

Attachment: Patch

Comment 34

[[e-mail] (gone - use [e-mail] instead)]

Seeking r/sr.

Comment 35

[[e-mail] (gone - use [e-mail] instead)]

I'm told by Pavlov this isn't supposed to be here, so it's all yours Blake.  

Comment 36

[Blake Ross]

The one in the cookie overlay is what you want.  That menuitem is inserted into 
the content area context menu when wallet and whatnot is installed.

Comment 37

[[e-mail] (gone - use [e-mail] instead)]

Attachment: Patch

Comment 38

[Blake Ross]

Thanks. r=blake

Comment 39

[Alec Flett]

sr=alecf

Comment 40

[[e-mail] (gone - use [e-mail] instead)]

Not sure if the entire bug is fixed or not, but I've checked in the wording
change to Mozilla0.9.  Thanks to all for speedy reviews.

Comment 41

[[e-mail] (gone - use [e-mail] instead)]

Since I "own" this bug, and Blake and I have fixed the bug as was filed, I'm 
taking the liberty to mark this as such.  Please file a new bug(s) if there are 
remaining issues.

Comment 42

[sairuh (rarely reading bugmail)]

unable to test due to bug 73848.

Comment 43

[[e-mail] (gone - use [e-mail] instead)]

My patch was just to change the wording though, not any functionality ;-)

Comment 44

[sairuh (rarely reading bugmail)]

first of all, my apologies for being so late/last-moment in commenting on
this bug. :(

anyhow, the phrase "Block Images from this Server" is actually
inaccurate. i agree with stephend, that this bug is just to change the
label, since the actual image blocking behavior is covered by bug
73848. however, the *context menu* behavior is misrepresented by the
current label --in the past, this item was supposed to block an individual
image on a given page, not all the images on a page from a given server.

to illustrate:

1. go to a page with more than one image such as
http://bugzilla.mozilla.org
2. bring up the context menu over the ant image and select "Block Images
from this Server"
3. reload the page [the ant image will remain, again due to bug 73848]
4. compare context menus: bring up the context menu over the ant image,
dismiss it, then bring up the context menu over the banner.

observe [expected]: the context menu for the ant image no longer has
"Block Images from this Server", whereas the one for the banner does have
the item.

so...reopening.

going to add a patch to rename the label to "Block this Image from
Loading". yes, it's sadly long-winded. imho, i actually thought the
previous phrase "Block Image from Loading" was clear enough --even
something like "Block this Image" would suffice. [i do agree that the
original "Block Image" was too vague.] just my $0.02...

Comment 45

[sairuh (rarely reading bugmail)]

Attachment: proposed patch with "Block this Image from Loading"

Comment 46

[[e-mail] (gone - use [e-mail] instead)]

Okay, my apologies, I was just told to rename.  Over to Sarah ;-)

Comment 47

[sairuh (rarely reading bugmail)]

/me doesn't have checkin privs ;)

Comment 48

[[e-mail] (gone - use [e-mail] instead)]

Sarah, if you get this reviewed by the module owner and super-reviewed, I can
check this in for you.

Comment 49

[sairuh (rarely reading bugmail)]

no rush to get this in, setting to 0.9.1.

mpt/others, any thoughts on the most recent patch? thx!

Comment 50

[[e-mail] (gone - use [e-mail] instead)]

Sarah, the images are coming from seperate servers though.

Mozilla banner location: http://www.mozilla.org/images/mozilla-banner.gif

Ant image location: http://bugzilla.mozilla.org/ant.jpg

So, they are essentially (to the code) different servers (it seems to block by
sub-domain then).

Is this expected behavior?

Comment 51

[sairuh (rarely reading bugmail)]

thanks for pointing that out, stephend!

in fact, i tried this out with a page with multiple images from the same server,
and the context menu remains the same for *all* images after i blocked one of them.

i didn't realize that's the expected behavior, now.

sheesh. sorry for all the noise! re-resolving this.

Comment 52

[sairuh (rarely reading bugmail)]

vrfy fixed using my debug mozilla build [n/a by default on commercial verif
bits].