Closed
Bug 568976
Opened 14 years ago
Closed 10 years ago
mail.trusteddomains does not display images from matching domains
Categories
(Thunderbird :: Preferences, defect)
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: der1way, Assigned: jik)
References
()
Details
(Whiteboard: [GS][similar change implemented w/ bug 457296 and bug 953426])
Attachments
(1 file)
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.
Comment 1•14 years ago
|
||
So this used to work in 2.x ?
Component: General → Preferences
QA Contact: general → preferences
Comment 2•13 years ago
|
||
According to http://kb.mozillazine.org/Privacy_basics_%28Thunderbird%29 it did.
Comment 3•13 years ago
|
||
The also exists on Ubuntu 10.10 64-bit running Thunderbird 5.0
Comment 4•13 years ago
|
||
(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
Comment 6•13 years ago
|
||
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.
Comment 7•13 years ago
|
||
(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"
Comment 8•13 years ago
|
||
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.
Assignee | ||
Comment 9•12 years ago
|
||
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
Assignee | ||
Comment 10•12 years ago
|
||
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 | ||
Updated•12 years ago
|
Assignee: nobody → jik
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee | ||
Comment 11•12 years ago
|
||
Attachment #598118 -
Flags: review?
Assignee | ||
Comment 12•12 years ago
|
||
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)
Comment 13•12 years ago
|
||
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.
Comment 14•12 years ago
|
||
There is a suite of content-policy tests here already: http://mxr.mozilla.org/comm-central/source/mail/test/mozmill/content-policy/
Comment 15•12 years ago
|
||
Although I am a bit concerned about overloading mail.trusteddomains, as that also appears to be used for spam settings.
Assignee | ||
Comment 16•12 years ago
|
||
(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.
Comment 17•12 years ago
|
||
(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 18•12 years ago
|
||
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+
Comment 19•12 years ago
|
||
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.
Assignee | ||
Comment 20•12 years ago
|
||
(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.
Comment 21•12 years ago
|
||
How can I apply this patch? Is it possible?
Assignee | ||
Comment 22•12 years ago
|
||
(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.
Updated•12 years ago
|
Whiteboard: [GS]
Comment 24•12 years ago
|
||
Can someone tell me if the patch is actually working ? I don't know how to compile TB...
Comment 25•12 years ago
|
||
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.
Assignee | ||
Comment 26•12 years ago
|
||
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.
Assignee | ||
Comment 27•12 years ago
|
||
(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.
Comment 28•12 years ago
|
||
(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 !
Comment 30•11 years ago
|
||
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
Assignee | ||
Comment 31•11 years ago
|
||
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.
Comment 32•11 years ago
|
||
**** 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 !
Assignee | ||
Comment 33•10 years ago
|
||
I believe there are changes on the trunk right now which make this bug moot.
Comment 34•10 years ago
|
||
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]
Comment 35•9 years ago
|
||
Why don't allow to add domain/subdomain of sender in the exception list?
Comment 36•6 years ago
|
||
I didn't understand why this won't be fixed. What is the alternative solution for "mail.trusteddomains" whitelisting?
Comment 37•6 years ago
|
||
(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.
Description
•