514739 - Remove "Block images from www.site.com"

Status: VERIFIED FIXED

(Firefox :: Menus, defect)

Description

[Justin Dolske [:Dolske]]

If you right click on an image, there's a context menu item to block images from the site. I think it's time for this feature to die...

* It's probably used most commonly for blocking ads, and AdBlock offers a far superior was of doing this. Not to mention a better way of dealing with image blocking, in general.

* It's easy to accidentally select this, and then have no idea why images on the site no longer load.

There's also a "Exceptions" button in Prefs -> Content, next to the "load images automatically". Not sure if we should look at removing that too, or if it's still useful. Probably needs to stay, unless we want to remove the backend support for image blocking entirely (which I'm wary of, because some users would wonder why it stopped working). Might be worth hiding the button if there are no sites with images blocked?

Comment 1

[Justin Dolske [:Dolske]]

Bug 513258 as one recent example of this causing a problem.

Comment 2

[(no longer active)]

(In reply to comment #0)
> Might be worth hiding the button if there are no sites with images blocked?

This UI can be used to enter exceptions manually, so hiding that button only if there are no exceptions doesn't make much sense.

Comment 3

[Justin Dolske [:Dolske]]

I think entering exceptions manually doesn't make much sense. :)

Comment 4

[chbok]

For a standard user, this menu bouton can easy be selected, and the user wouldn't find in the options how to re-enable the images. In my office, i just helped someone who encountered the issue.

I don't understand how it's so simple to be disabled images without a clear warning.

Comment 5

[Christian Kujau]

Please don't remove this feature. I always considered the built-in "block images" function to be an outstanding feature. It's simple, everybody can use it and does not require 3rd party add-ons. Especially the aforementioned AdBlock, which might be good for power-users but is far too complex for everyone else. I agree, things like 513258 need(ed) to be addressed - and they have.

However, I see that this feature vanished already from the FF 3.6 Windows builds. How comes?

Comment 6

[Wladimir Palant]

Note that this menu item is a major support issue for Adblock Plus. People commonly click this item, see that too many images are blocked but cannot get them back - and they typically blame that on Adblock Plus, to the point that they complain in my forum about Adblock Plus messing up their browser despite being uninstalled. It got so bad that Adblock Plus 0.7 started hiding that menu item for users who don't have anything in their image exceptions list yet - if they didn't use that feature yet chances are that they only get confused by it. I don't think I've ever heard a complaint in the four years this code is in, the number of support requests due to inexplicably missing images went down considerably however. So while the content blocker might be a useful built-in feature, its UI definitely needs to be rethought.

Comment 7

[Matthias Versen [:Matti]]

Many users block images by a mistake. There are for example many reports in the openstreetmap.org Forum about users which blocked one of the three tile servers.
The users usually never find the place to remove the block and they often don't recognize the infobar that warns you about the blocking.

Comment 8

[atzaus]

> it's still useful. Probably needs to stay, unless we want to remove the backend
> support for image blocking entirely (which I'm wary of, because some users
> would wonder why it stopped working).

IMO removing the back-end support makes more sense since this feature causes more headache than it's worth. Anyone who needs image blocking (majority of the usage is in ad blocking) will find adblock more convenient

Comment 9

[Christian Kujau]

Well, maintaing the "block images from site"-blocklist is kinda hard for non-powerusers, I understand. But I found/find it /very/ cool from firefox to finally recognize the user's wishes and block images on request and providing this ability *out of the box*. Whooha! Without installing plugins. But I admit, I have to come to terms with the fact that Firefox is no longer being developed for these power-users, appreciating idealistic features like this :-|
If it's not too hard to do: hide this feature by default, but can this be enabled again via some about:config knob?

Thanks,
C.

Comment 10

[Frank Yan (:fryn)]

Attachment: patch v1 (removes all UI-related code+tests w/o removing the ability to block images)

you can still access this from 'view page/media/image info', so s'all good.

Comment 11

[Frank Yan (:fryn)]

Attachment: patch v2 (removes all UI code+tests for 'block images from ...' context menu item)

forgot to remove stuff from test_contextmenu.html

files touched:
* browser/base/content/browser-context.inc
* browser/base/content/nsContextMenu.js
* browser/base/content/test/test_contextmenu.html
* browser/components/privatebrowsing/test/browser/Makefile.in
* browser/components/privatebrowsing/test/browser/browser_privatebrowsing_contextmenu_blockimage.js (deleted)
* browser/locales/en-US/chrome/browser/browser.dtd
* browser/locales/en-US/chrome/browser/browser.properties
* testing/mozmill/tests/firefox/testPrivateBrowsing/testDisabledPermissions.js

Comment 12

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 456767 [details] [diff] [review]
patch v2 (removes all UI code+tests for 'block images from ...' context menu item)

Might want to pass the mozmill change past someone who knows how to run them  and can confirm that it passes (Henrik?), since I think they don't run on tinderbox yet?

Comment 13

[:Gavin Sharp [email: [e-mail]]]

Bonus points for putting this code in an extension on AMO for people who want to preserve this functionality :)

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #12)
> Might want to pass the mozmill change past someone who knows how to run them 
> and can confirm that it passes (Henrik?), since I think they don't run on
> tinderbox yet?

The Mozmill tests in m-c are only a copy of our original tests and mostly out of date right now. Frank, it would be great if you can create a separate patch against our test repository, which you can find here: http://hg.mozilla.org/qa/mozmill-tests/.

Removing the test makes sense, given the fact that there is no entry anymore and you are testing it also.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Please remove the Mozmill test changes first. Removing checkin needed keyword for now.

Comment 16

[Frank Yan (:fryn)]

(In reply to comment #15)
> Please remove the Mozmill test changes first. Removing checkin needed keyword
> for now.

working on it :)

Comment 17

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #14)
> The Mozmill tests in m-c are only a copy of our original tests and mostly out
> of date right now.

Can we remove them, then?

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #17)
> > The Mozmill tests in m-c are only a copy of our original tests and mostly out
> > of date right now.
> 
> Can we remove them, then?

They will be used in the near future and simply need an update from our external repository. See bug 516984 for the integration work.

Comment 19

[Frank Yan (:fryn)]

Attachment: patch for mozmill-tests tree [checked-in]

Here you go, Henrik. :)

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 457026 [details] [diff] [review]
patch for mozmill-tests tree [checked-in]

Thanks. I will push it once the other patch has been landed. You can remove the patch for the Mozmill test from the mozilla-central patch.

Comment 21

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

http://hg.mozilla.org/mozilla-central/rev/4dd7bbd2ebeb

Comment 22

[Tanner M. Young [:tmyoung]]

I assume with this we should also remove the functionality in Page Info (particularly considering Bug 416557).  Correct?

Comment 23

[Frank Yan (:fryn)]

(In reply to comment #22)
> I assume with this we should also remove the functionality in Page Info
> (particularly considering Bug 416557).  Correct?

My discussion with the UX team did not bring up anything about removing the Page Info checkbox, so I didn't remove it, but I will ask them.

Comment 24

[Tanner M. Young [:tmyoung]]

It would make sense to, and since I already planned to make changes to it, I will just remove it unless someone objects.

I wouldn't be surprised if they forget that Page Info used it ;)

Comment 25

[Justin Dolske [:Dolske]]

I think we should go ahead and remove the Page Info UI, along with the ability to _add_ exceptions in the Prefs UI.

Let's take this work (and discussion) to a followup bug, this one is done.

Comment 26

[Tanner M. Young [:tmyoung]]

I have Bug 416557 for the Page Info removal, but I didn't

Comment 27

[Tanner M. Young [:tmyoung]]

...Correction from the comment that was cut off...

I also filed Bug 578915 for the Preferences UI.

Comment 28

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 457026 [details] [diff] [review]
patch for mozmill-tests tree [checked-in]

Landed the mozmill patch on our default branch:
http://hg.mozilla.org/qa/mozmill-tests/rev/b46735355322

Thanks for taking care of it!

Comment 29

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with  Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; en-US; rv:2.0b2pre) Gecko/20100718 Minefield/4.0b2pre

Updated the following Litmus test to make sure no test mentions that menu item anymore: https://litmus.mozilla.org/show_test.cgi?id=11142

If setting in-litmus-/in-testsuite- the right state please correct it.

Comment 30

[:aceman]

I hope when this really gets removed from FF the AdBlock plus method is similarly easy to use. And I hope it also offers the opposite functionality - disabling images globally and allowing them on specific sites. And of course the possibility to disable the rest of the ABP functionality and to only use image blocking. I don't seem to like the direction this bug is moving, but it seems already checked in.

Comment 31

[Michael Shigorin]

PS: replacing convenient basic functionality with a (missing) reference to a pretty complex extension (that I don't consider requisite, for example) is indeed a way to go if one wishes to spend half of his time along the lines of "slow?  and if run without extensions?  fast?  ah, then try removing them all and adding only those needed".

Maybe I'm not representative but hope you at least did some user behaviour research so this "bugfix" actually helped someone, Justin.