Closed Bug 568976 Opened 14 years ago Closed 10 years ago

mail.trusteddomains does not display images from matching domains

Categories

(Thunderbird :: Preferences, defect)

Product:
Thunderbird
Component:
Preferences
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX

People

(Reporter: der1way, Assigned: jik)

References

(
URL
)

Details

(Whiteboard: [GS][similar change implemented w/ bug 457296 and bug 953426])

Attachments

(1 file)

Patch to implement the suggested behavior
1.51 KB, patch
standard8
: feedback+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.9) Gecko/20100317 Thunderbird/3.0.4

In thunderbird 3.0.4, adding entries to mail.trusted domain, does not force it to show to display remote images, as it is supposed to.


Reproducible: Always

Steps to Reproduce:
1. use options/config editor to add a domain to mail.trusteddomains
2. close options and restart thunderbird
3. examine emails from the domain you added, and see they are still not showing remote image content.
Actual Results:  
No change in display behavior for html email messages from those domains.  It still offers "click here to display remote content"

Expected Results:  
It should just automatically show all the remote content of email messages from the indicated domains.

In general, the privacy is too onerous to screw around with on a address by address basis, and even this domain by domain system is broken.

Please consider making remote image display by mail folder - so I can tell thunderbird to display images in all the mail messages within this folder.
So this used to work in 2.x ?
Component: General → Preferences
QA Contact: general → preferences
The also exists on Ubuntu 10.10 64-bit running Thunderbird 5.0
(In reply to comment #3)
> The also exists on Ubuntu 10.10 64-bit running Thunderbird 5.0

The bug*
Hm, AFAIK the domains of the images should be added here, which is not necessary the one from the sender, take facebook as an example:

* email from hmhmhm@facebookmail.com
* images from (look into the source, searching for src)
** http://profile.ak.fbcdn.net
** https://fbcdn-profile-a.akamaihd.net
** http://www.facebook.com
** http://www.facebookmail.com

so mail.trusted domain=vbcdn.net,akamaihd.net,facebook.com,facebookmail.com
WITHOUT SPACES ANYWHERE IN THE VALUE should work
According to http://kb.mozillazine.org/Privacy_basics_%28Thunderbird%29:
One would "create a mail.trusteddomains setting that specifies what e-mail domains it should automatically display remote images for".

It makes no sense to have a list of trusted image domain sources.  It makes more sense to have a list of trusted senders by email or domain.
(In reply to Nathan J. Brauer from comment #6)
> According to http://kb.mozillazine.org/Privacy_basics_%28Thunderbird%29:
> One would "create a mail.trusteddomains setting that specifies what e-mail
> domains it should automatically display remote images for".
> 
> It makes no sense to have a list of trusted image domain sources.  It makes
> more sense to have a list of trusted senders by email or domain.

We already have a way of trusting senders by email address; you can click the "Always load remote content from <address>" link in the remote content alert to add the sender to your list. This isn't perfect; if an attacker fakes the "From" address on the email to be one you trust, you'll load their images.

There are use cases for trusted domains too; for example, "friends send me <photo-site> images all the time, so automatically display any embedded images from *.<photo-site>.org"
I'm aware, Irving.  I was replying to V.L. from comment #5 who said "the domains of the images should be added here".  I'm saying this is unintelligent.  mail.trusteddomains should be a list of trusted sender-domains.
Wow. Useful feature, regression, nobody appears to have looked at it in over a year and a half since it was filed.

I guess I'll take a swing at it, since I just ran into it.
Keywords: regression
I agree with comment 6 that the behavior that makes sense is for mail.trusteddomains to contain a list of domains to be matched against email senders to remote images in mail from those senders to automatically be displayed.

However, I agree with comment 5 that the behavior that makes sense is not in fact the current behavior. In fact, the current behavior is described in comment 5, i.e., to match against mail.trusteddomains not the domain of the sender's email address, but rather the domains of the URLs embedded in the message.

I've spent hours poring over the source code ranging all the way back to Thunderbird 1.5, and testing this functionality on Thunderbird 1.5.0.9, 2.0.0.0, and the current trunk. As far as I can tell, although I _thought_ that at some point in the past the functionality was as described in comment 6, in fact it appears that the functionality has _always_ been as described in comment 5. If it sometimes looked like comment 6, it was only because there were sometimes URLs in the message whose domains were the same as the sender's email address's domain.

Interestingly, there _is_ code in the current Thunderbird code base which checks the sender address's domain against mail.trusteddomains as suggested in comment 6, but it doesn't do that for the purpose of determining whether to display images, but rather for the purpose of determining whether the message is spam. See nsSpamSettings::CheckWhiteList in mailnews/base/src/nsSpamSettings.cpp.

In short, despite how many people _thought_ mail.trusteddomains behaves, in fact making it behave that way would be new functionality. I think it would be _correct_ functionality, but it's not a regression that it doesn't behave that way.
Keywords: regression
Assignee: nobody → jik
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 598118 [details] [diff] [review]
Patch to implement the suggested behavior

Here's a patch to implement the suggested behavior, which many people thought was already the current behavior but actually wasn't.

Asking Mark Banner to review it because he has worked in this area of the code.

I tried to write a JavaScript unit test for this, but as far as I can tell nsMsgContentPolicy.cpp's interfaces are not exported into JavaScript, so I don't think I can do that. If I'm wrong, somebody please tell me how I would access this class from JavaScript.

Alternatively, if there's a way to write a unit test without having to access the class from JavaScript, please enlighten me.
Attachment #598118 - Flags: review? → review?(mbanner)
How about a Mozmill test instead? You could just make an appropriate email and check for the presence of the remote images bar in the UI.
There is a suite of content-policy tests here already:

http://mxr.mozilla.org/comm-central/source/mail/test/mozmill/content-policy/
Although I am a bit concerned about overloading mail.trusteddomains, as that also appears to be used for spam settings.
(In reply to Mark Banner (:standard8) from comment #15)
> Although I am a bit concerned about overloading mail.trusteddomains, as that
> also appears to be used for spam settings.

It's already overloaded, considering that it is being used both for spam whitelisting and for determining which URLs to display without prompting.

I don't think either of these usages is particularly well-documented. And of course the preference itself, since it is not accessible through the normal UI, is completely undocumented in the KB, because of their policy of not allowing anything to be documented that isn't accessible through the UI.

Given all this, I see little harm in modifying the code to make the setting do what most people who knew about it thought it was already supposed to do.

I also see little harm in it because I think the way I'm proposing for it to be used is consistent with the other ways it is already being used. I don't see much at all to gain from having a separate preference for this particular bit of functionality.

I'll try to put together a mozmill test as soon as I can.

Somebody want to give me a hint for how to run my test (on Linux) once I've got it written, without running all the others? I can probably figure it out myself with some trial and error and googling, but if somebody else here knows off the top of their head, it'd be useful.
(In reply to Jonathan Kamens from comment #16)
> Somebody want to give me a hint for how to run my test (on Linux) once I've
> got it written, without running all the others? I can probably figure it out
> myself with some trial and error and googling, but if somebody else here
> knows off the top of their head, it'd be useful.

https://developer.mozilla.org/en/Thunderbird/Thunderbird_MozMill_Testing#runtests
Comment on attachment 598118 [details] [diff] [review]
Patch to implement the suggested behavior

I think the idea generally looks fine, but for content policy changes we definitely require tests, so f+ for now. If you have any problems writing them let me know.
Attachment #598118 - Flags: review?(mbanner) → feedback+
This overlaps with the fix I've been working on in Bug 457296; the WIP patch there also supports whitelisting both individual accounts and sending domains, but uses the Permissions database (instead of the mail.trusteddomains preference) to store the white list.
(In reply to Irving Reid (:irving) from comment #19)
> This overlaps with the fix I've been working on in Bug 457296; the WIP patch
> there also supports whitelisting both individual accounts and sending
> domains, but uses the Permissions database (instead of the
> mail.trusteddomains preference) to store the white list.

I'm happy to defer to that change if it is going to land soon, but if not, I'd like to get this one done in the interim. And yes, I know, what's blocking this one is me writing test cases; I will get to it as soon as I can.
How can I apply this patch? Is it possible?
(In reply to westmoquette from comment #21)
> How can I apply this patch? Is it possible?

Not without recompiling Thunderbird from source code. If you don't know how to figure out how to do that on your own, you probably can't afford it. :-)

Sorry I haven't had time yet to put together a test case.
Can someone tell me if the patch is actually working ? I don't know how to compile TB...
I just do some search on GSFN and found actually two thread with 20 votes for this feature (see: https://getsatisfaction.com/mozilla_messaging/tags/bug_568976 ). 
From a French discussion on Geckozone, this is particularly annoying with Facebook's notifications, because they use a different address each time, so you always have to allows them. 
So it would be great if the patch here could be improved and applied ASAP.
I don't disagree that it's an important feature, and if I had time to work on a unit test for it, I would, but I simply don't. If someone else has time to come up with a unit test, I (and others) would be grateful, but otherwise, it's just going to have to wait until I have more time to spend on it. I agree with others that this is not the kind of change that should be checked in without a test.
(In reply to gastonwazeef from comment #24)
> Can someone tell me if the patch is actually working ? I don't know how to
> compile TB...

Yes, it works.
(In reply to Jonathan Kamens from comment #27)
> (In reply to gastonwazeef from comment #24)
> > Can someone tell me if the patch is actually working ? I don't know how to
> > compile TB...
> 
> Yes, it works.

Thank you so much !
I can't wait for its adding in final release !
the patch here is now one year old. 

Do you think there is still a lot to do to finish the work here? We still have users asking for this to be fixed, see http://forums.mozfr.org/viewtopic.php?f=4&t=104989&p=722729&e=722729 for instance
I have not had time to write a unit test for it.

I do not know when I will have time. The unit test process is cumbersome and time-consuming for me because I do it infrequently. On top of that, I have a more-than-full-time job and five kids, and I maintain several TB add-ons including one that has over 28,000 active users. I do not have a lot of free time. :-/

If someone else who is more adroit with the unit test process were to step in and write the unit test, I would certainly appreciate it, as would the other people who want to see this patch make it into TB.

Otherwise, it will just have to wait until I have time.
**** off of waiting, I just switched to zimbra desktop and I'm happy. Mozilla is not a good software maker anymore, I don't use Firefox anymore as well. Mozilla = slow apps, stupid 'evolutions', etc. Fx and TB 3.x was better than today...
Wake up guys !
I believe there are changes on the trunk right now which make this bug moot.
Yes, after bug 457296 and bug 953426 it's rather moot for tb 31+.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
Whiteboard: [GS] → [GS][similar change implemented w/ bug 457296 and bug 953426]
Why don't allow to add domain/subdomain of sender in the exception list?
I didn't understand why this won't be fixed.

What is the alternative solution for "mail.trusteddomains" whitelisting?
(In reply to NaTong from comment #35)
> Why don't allow to add domain/subdomain of sender in the exception list?

Because there's now UI for doing what was requested. 
(In reply to Thomas Stein from comment #36)

> I didn't understand why this won't be fixed.
> 
> What is the alternative solution for "mail.trusteddomains" whitelisting?

Click the options button that's displayed when you get a message with remote content. From there you can specify what to allow/disallow. Or go through Options | Privacy | Mail Content
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: