522019 - thunderbird shows blocked content when composing reply

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Mads Kiilerich]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.3) Gecko/20090909 Firefox/3.5.3-1.fc11 Firefox/3.5.3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.4pre) Gecko/20090922 Fedora/3.0-2.7.b4.fc11 Thunderbird/3.0b4

It is nice that thunderbird blocks URL content by default when viewing mails.

But when a message with blocked content is replyed to or forwarded then the editor apparently loads the URLs by default. It shouldn't.

Isn't this a minor privacy issue?

Reproducible: Always

Comment 1

[Mark Banner (:standard8)]

From the code and previous bugs, if this is verified then this is an issue we should fix for TB 3. I'll try and verify what is happening soon.

Comment 2

[Mark Banner (:standard8)]

Confirming, this is a regression from bug 424767 - I have a proposed fix, though would like to try and write a mozmill test for it first.

Comment 3

[Mark Banner (:standard8)]

Attachment: [checked in] Proposed fix

Simplish fix, but I want more than one pair of eyes on it. This fixes the issue.

What is happening is that when we load the message into the compose window, the editor element (which acts like a browser element) actually has a requesting location (src) of about:blank.

This was fine until bug 424767 came along and blanket-allowed about: as the requesting location.

Therefore this patch checks to see if about:blank is the location and doesn't allow that, and therefore the content policy falls back on the existing code that does similar checks to message window - if remote content is allowed for the sender of the original email or if remote content was just allowed anyway.

I have a test case that is in progress for this but I wanted to get this moving anyway so that Dan has time to review before he goes travelling.

Comment 4

[David :Bienvenu]

Comment on attachment 406727 [details] [diff] [review]
[checked in] Proposed fix

this looks reasonable to me - I'll try running with it to see if anything breaks.

Comment 5

[Mark Banner (:standard8)]

Attachment: [checked in] MozMill Test

Test case for this bug. This uses the video element as a test case for not allowing remote content in emails (video had the easiest way of detecting no-load allowed and hence was easier than images for this test, however, given we treat both the same it doesn't matter).

Don't be scared by the size of the patch, most of it is just a short video.

There's one element I had to add an id to so that mozmill could easily hook into the button as well.

Dan if you haven't got time for this please feel free to pass the review to David or someone else.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 406727 [details] [diff] [review]
[checked in] Proposed fix

r=dmose

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 407111 [details] [diff] [review]
[checked in] MozMill Test

Dan's now travelling, David could you take a look at this?

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 406727 [details] [diff] [review]
[checked in] Proposed fix

Checked in: http://hg.mozilla.org/comm-central/rev/520db1c594d9

Comment 9

[Mark Banner (:standard8)]

This is now fixed, I'll check in the unit test once I get review for it.

Comment 10

[David :Bienvenu]

Comment on attachment 407111 [details] [diff] [review]
[checked in] MozMill Test

nit - license year is 2001 

+ * Portions created by the Initial Developer are Copyright (C) 2001

Comment 11

[Mark Banner (:standard8)]

Comment on attachment 407111 [details] [diff] [review]
[checked in] MozMill Test

Checked in (first revisions were incorrect ones, backed out in the second):

http://hg.mozilla.org/comm-central/rev/ea7c702cf273
http://hg.mozilla.org/comm-central/rev/558474146a03
http://hg.mozilla.org/releases/comm-1.9.1/rev/7ea2d7831a65
http://hg.mozilla.org/releases/comm-1.9.1/rev/ff0c9980fffb

Comment 12

[Andrew Sutherland [:asuth] (he/him)]

The windows mozmill comm 1.9.1 test box seems to reliably fail on this test.

Comment 13

[Mark Banner (:standard8)]

(In reply to comment #12)
> The windows mozmill comm 1.9.1 test box seems to reliably fail on this test.

As the Mac box intermittently fails (i.e. it does pass sometimes), I'm assuming it is a timeout issue, therefore I've pushed a change to increase the timeout to 100 seconds on just the 1.9.1 branch. This should hopefully eliminate if it is due to the timeout being too short or if it is due to the notification not actually being (consistently?) received.

http://hg.mozilla.org/releases/comm-1.9.1/rev/e7df22acf0d1

Comment 14

[Mark Banner (:standard8)]

Urgh, the other option is that by the time mozmill has decided the window is open, the notification has actually fired, and this is a bogus error. They haven't cycled yet, so I'll see what the current patch does first.