If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Don't fetch thumbnails in Add-ons manager if load images automatically is disabled

RESOLVED FIXED in mozilla1.9.1b1



Add-ons Manager
9 years ago
9 years ago


(Reporter: Cww, Assigned: Natch)


Windows XP

Firefox Tracking Flags

(Not tracked)



(1 attachment, 3 obsolete attachments)



9 years ago
Bug 446301 got me thinking about users who want to minimize the amount of traffic that Firefox generates.  Right now, one thing we tell them is to disable image loading in in the Preferences, Content pane.  However, the addons manager, get addons pane does not respect this preference.

Yes I understand that these thumbnails are tiny and this bug is minor but it's a preference that's there for a reason and we might as well respect it.
This sounds like a valid thing to handle
Ever confirmed: true
Whiteboard: [good first bug]

Comment 2

9 years ago
Created attachment 335830 [details] [diff] [review]
Patch using empty string url
Attachment #335830 - Flags: review?


9 years ago
Attachment #335830 - Flags: review? → review?(mano)


9 years ago
Attachment #335830 - Flags: review?(mano) → review?(gavin.sharp)
Attachment #335830 - Flags: review?(gavin.sharp) → review?(dtownsend)
Attachment #335830 - Flags: review?(dtownsend) → review-
Comment on attachment 335830 [details] [diff] [review]
Patch using empty string url

This is not the right place for this code. The image blocking should be based off the actual blocking policy, not just the default, this means checking the content policy for each thumbnail, either in displaySearchResults or in the search result binding where it pulls the thumbnail.

Comment 4

9 years ago
Created attachment 336406 [details] [diff] [review]
Patch in extensions.xml

Here's the updated patch according to your request, it checks through nsIContentPolicy in the constructor of the thumbnail.

Hope this is right.
Attachment #336406 - Flags: review?(dtownsend)
Comment on attachment 336406 [details] [diff] [review]
Patch in extensions.xml

Yeah this looks great, just a minor style nit then it will be ready for checkin.

>+      <method name="_imageBlocked">
>+        <parameter name="container"/>
>+        <body>
>+          var uri = Components.classes["@mozilla.org/network/io-service;1"]
>+                              .getService(Components.interfaces.nsIIOService)
>+                              .newURI(container.getAttribute("thumbnailURL"),
>+                               container.ownerDocument.characterSet, null);

Line up the arguments with the arguments above to make it clear they are a part of the method call.

>+          var contentPolicy = Components.classes["@mozilla.org/layout/content-policy;1"]
>+                                        .getService(Components.interfaces.nsIContentPolicy);
>+          if (contentPolicy.shouldLoad(contentPolicy.TYPE_IMAGE,
>+                                       uri, container.ownerDocument.documentURIObject,
>+                                       container, null, null) != contentPolicy.ACCEPT)

You can avoid the if statement and just |return contentPolicy... != contentPolicy.ACCEPT|
Attachment #336406 - Flags: review?(dtownsend) → review+

Comment 6

9 years ago
Created attachment 336523 [details] [diff] [review]
nits addressed

Created attachment 336623 [details] [diff] [review]
patch for checkin

Just revved for a couple of new style nits, dropped the surrounding brackets and added a semicolon at the end of the line.
Attachment #335830 - Attachment is obsolete: true
Attachment #336406 - Attachment is obsolete: true
Attachment #336523 - Attachment is obsolete: true
Landed: http://hg.mozilla.org/mozilla-central/rev/d8064eff0a17

Thanks for the patch.
Assignee: nobody → highmind63
Last Resolved: 9 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug]
Target Milestone: --- → mozilla1.9.1b1
You need to log in before you can comment on or make changes to this bug.