# Inserting a link to a [network][image] file into a message inserts the physical file!

VERIFIED FIXED

VERIFIED FIXED
18 years ago
11 years ago

## Tracking

### (Depends on 1 bug, Blocks 1 bug, 4 keywords)

Trunk
---
Dependency tree / graph
Bug Flags:
 blocking1.8b2 - blocking1.8b5 - blocking1.8.1.1 -

## Attachments

### (7 attachments, 8 obsolete attachments)

 12.33 KB, patch cavin : review+ Details | Diff | Splinter Review 49.07 KB, image/jpeg Details 29.97 KB, patch iann_bugzilla : review+ Bienvenu : superreview+ mconnor : approval1.8.1+ Details | Diff | Splinter Review 1.15 KB, patch neil : review+ Bienvenu : superreview+ Details | Diff | Splinter Review 3.02 KB, patch neil : review+ Details | Diff | Splinter Review 9.55 KB, patch iann_bugzilla : review+ iann_bugzilla : superreview+ mscott : approval-thunderbird2+ Details | Diff | Splinter Review 24.23 KB, patch iann_bugzilla : review+ iann_bugzilla : superreview+ mscott : approval-thunderbird2+ mscott : approval1.8.1.1+ Details | Diff | Splinter Review
Reporter

### Description

18 years ago
When inserting a link in an html message to a local network file
messenger insert the link right but insert the file too...
I tried to reduce the size of my email and beeeeep I failed !

--
arakeis

Info : Build 2002031903 / W2K SP2 / French

### Comment 1

18 years ago
I can understand your frustation Arnaud but network file are not considered as
remote attachment therefore we send the file as well in order to be sure the
recipient will be able to access it. Like we do with any local file.

I don't really know yet how we can fix that dilemna. Maybe we could ask the user
what to do in that specific case which hopfully should not append so often.
Severity: normal → enhancement
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Target Milestone: --- → Future
Reporter

### Comment 2

18 years ago
Jean-François wrote :
>network file are not considered as
>remote attachment

That's my point: a link to a file in a message isn't an attachment !
maybe I wasn't clear enough ? Do you think I should start some kind
of a poll on mozillazine ?

--
Arakeis

### Comment 3

18 years ago
Why do we send the file data when you put a link to it into your message body?
to let the recipient be able to use the link else it's useless! We do that only
with file:// urls but not for http:// or ftp:// urls.

The problem is that network file use a regular file url (file://) therefore
there is not an easy way to know if a file is local or remote. Sure on Windows
we can look for a double back slash at the beginning but on other platform like
Mac or Unix, it's another story...

BTW, did you try just typing the url into the message body instead of inserting
a link tag? That might does just do what you want.

Feel free to start a poll if you want on mozillazine or a dicussion on
netscape.public.mozilla.mailnews.
Reporter

### Comment 4

18 years ago
As I understand it, because sending a "pure" link for a local file to somewone
won't be usable for the receiver, the mailnews module translate it into a mix
of attachement and link. This is definitivly not the good solution (IMHO),
the function attachement exit on it's own, if people don't understand what a
link is, let them fail and redo the manipulation the right way !
Anyway thank you for your quick response (even if the response wasn't
the one I woped for)

--
Arakeis

### Comment 5

18 years ago
Adding keyword privacy. Just imagine you want to ease the work of someone else
by using a link to *their* file on *their* computer (i.e. /etc/passwd on the
network) then that's the solution I would choose and it never ever would come to
my mind that this could attach *my* /etc/passwd file.

### Updated

18 years ago
Keywords: privacy

### Comment 6

18 years ago
That's a good point! I think we need to warm/ask the user to attach or not the
file when inserting a link into the message body. In fact the best way would be
do do it during the send process as there is many way to provoke a file to be
automatically attached.
Target Milestone: Future → mozilla1.2alpha
Reporter

### Comment 7

18 years ago
the ultimate solution ... would be :
1/ ask if user want's his file attached,
2/ if anwser is yes : show the attachment in attachment box,

this allow the user to correct an eventual typo when answering,
by removing the file.



### Updated

18 years ago
OS: Windows 2000 → All
Hardware: PC → All

### Comment 8

18 years ago
In MS Outlook we often send links to "local" files that are on network shares.
We do not want to attach the file, but send the link. The same think should be
possible for Mozilla/Netscape.

This behavior is _not_ intuitive and the user risks sending private/confidential
information unknowingly. 
Reporter

### Comment 9

18 years ago
Sad that correction of a incorrect and secret behaviour is considered as a
enhancement...
Assignee

### Comment 10

18 years ago
Actually NS4.7x let you do this without attaching a file, so should really be
marked as 4xp too. In fact if someone using NS4.7x sends u a file:// link,
mozilla doesn't let you open it - I'll log a separate bug on this.
Assignee

### Comment 11

18 years ago
Separate bug as mentioned in comment 10 is bug 135830

### Comment 12

17 years ago
this is similar but different that bug 128693
Target Milestone: mozilla1.2alpha → mozilla1.1beta

### Comment 13

17 years ago
The current workaround is to add the following attribute to the link (you can
moz-do-not-send = true

I good solution would be to add a checkbox item in the link property dialog to
let the user chose to attach or not the file linked to the message.
Target Milestone: mozilla1.1beta → Future

### Comment 14

17 years ago
*** Bug 128693 has been marked as a duplicate of this bug. ***

### Updated

17 years ago
Depends on: 135830

### Updated

17 years ago
Whiteboard: have fix
Target Milestone: Future → mozilla1.3beta

### Comment 15

17 years ago
Posted patch Proposed fix, v1 (obsolete) — Splinter Review
The fix add 2 functionalities:

1) you can setup a user preference in prefs.js (currently named
"mail.compose.dont_attach_source_of_remote_file_url") to tell Mozilla do not
attach the source or a local network file.

2) I added a checkbox "Attach source of [image | link] to message" in the
Insert Image and Insert Link dialog. The value of the check box depend on the
nature of the url but the user can overwrite the default behavior by changing
its state. When the use click on the checkbox, the attribute
moz-do-not-send=true|false is added to the embedded object.

### Comment 16

17 years ago
Comment on attachment 108264 [details] [diff] [review]
Proposed fix, v1

r=cavin.
Attachment #108264 - Flags: review+

### Comment 17

17 years ago
This needs more work for the XUL/JS part:
1. JS warning:
+        mozDoNotSend = globalElement.getAttribute(gMOZDONOTSEND);
should be:
+        var mozDoNotSend = globalElement.getAttribute(gMOZDONOTSEND);

2. You should add some code that reads the current setting when user edits an
image or link (e.g., double click to bring up appropriate dialog). Currently,
the new checkbox shows last state, not the state of "moz-do-not-send" set when

3. Shouldn't there be some UI in prefs so user can set the new pref?

4. I tested the patch and it seems to work, but I found some problems that are
probably not because of this patch, but should be addressed:
1) Lots of asserts! Most common were at :
generate_encodedwords(char * 0x04c702d8, const char * 0x05229298, char 81, char
* 0x05438ef8, int 116, int 0, int 72, int 1) line 280 + 32 bytes
apply_rfc2047_encoding(const char * 0x05436898, int 0, const char * 0x05229298,
int 0, int 72) line 795 + 37 bytes
MIME_EncodeMimePartIIStr(const char * 0x05436898, int 0, const char *
0x05229298, const int 0, const int 72) line 1164 + 25 bytes
nsMimeConverter::EncodeMimePartIIStr_UTF8(nsMimeConverter * const 0x036bcc00,
const char * 0x05436898, int 0, const char * 0x05229298, int 0, const int 72,
char * * 0x0012efac) line 159 + 25 bytes
nsMsgI18NEncodeMimePartIIStr(const char * 0x05436898, int 0, const char *
0x05229298, int 0, int 1) line 399 + 54 bytes
mime_generate_attachment_headers(const char * 0x053b9ac0, const char *
0x00000000, const char * 0x05423da8, const char * 0x0542b5f0, const char *
0x05436580, const char * 0x054365c8, const char * 0x05436898, const char *
0x054366b0, int 0, int 0, const char * 0x05436610, const char * 0x05455b00, int
0, const char * 0x04c703d0, int 0) line 814 + 28 bytes
nsMsgComposeAndSend::PreProcessPart(nsMsgAttachmentHandler * 0x0541ef74,
nsMsgSendPart * 0x053b9690) line 1308 + 115 bytes
nsMsgComposeAndSend::GatherMimeAttachments(nsMsgComposeAndSend * const
0x053fe2c8) line 1149
nsMsgAttachmentHandler::UrlExit(unsigned int 0, const unsigned short *
0x00000000) line 1222 + 41 bytes
FetcherURLDoneCallback(unsigned int 0, const char * 0x052297c8, const char *
0x00000000, int 341, const unsigned short * 0x00000000, void * 0x0541f010) line
479 + 16 bytes
nsURLFetcher::OnStopRequest(nsURLFetcher * const 0x0545dc0c, nsIRequest *
0x0545e230, nsISupports * 0x00000000, unsigned int 0) line 322 + 52 bytes
nsDocumentOpenInfo::OnStopRequest(nsDocumentOpenInfo * const 0x0545e308,
nsIRequest * 0x0545e230, nsISupports * 0x00000000, unsigned int 0) line 257
nsFileChannel::OnStopRequest(nsFileChannel * const 0x0545e23c, nsIRequest *
0x0545e444, nsISupports * 0x00000000, unsigned int 0) line 597
nsOnStopRequestEvent::HandleEvent() line 213

and during sendind of message at:
NS_CheckThreadSafe(void * 0x002c1090, const char * 0x06664524) line 533 + 34 bytes
nsURLFetcher::Release(nsURLFetcher * const 0x0545dc08) line 64 + 76 bytes
nsCOMPtr<nsIInterfaceRequestor>::~nsCOMPtr<nsIInterfaceRequestor>() line 491
nsFileTransport::~nsFileTransport() line 306 + 89 bytes
nsFileTransport::scalar deleting destructor'(unsigned int 1) + 15 bytes
nsFileTransport::Release(nsFileTransport * const 0x0545e440) line 312 + 147 bytes

2) I tested various combinations of including images with and without being
attached, and links with and without being attached. When there is 1 each of
both images and links, I crashed frequently (I think the order of them in
the page may be important?) It wasn't 100% reproduceable.



### Comment 18

17 years ago
cmanskey, thank you for your testing. Can you just tell me at which point you
were crashing? during the edition (when open the dilog, close it, etc) or did
you send the message?

### Comment 19

17 years ago
todo list:

1) mailbox url should turn checkbox on by default
2) need to initialize the checkbox when opening dialog
3) check cmanskey warning and crash
4) get ok from UI folk

### Comment 20

17 years ago
*** Bug 177890 has been marked as a duplicate of this bug. ***

### Updated

17 years ago
Depends on: 118038

### Comment 21

17 years ago
I have addressed most of cmanske comments: no, we will not have a UI for the
new pref, this is only a power user feature. Also, I wasn't able to reproduce
the crash!

This patch also fix couple other issue I found while doing more intensive test:

1) Editor wasn't giving the list of links to remote file. That was preventing
the user from forcing to attach a remote link.

2) When sending a link to a remote attachment and forcing the source to be
attached, we could not browse this link with mozilla because of a bug in mime.

3) I have changed the pref name to

WARNING: if you want to test this patch, make sure you have also the fix for
118038
Attachment #108264 - Attachment is obsolete: true

### Comment 22

17 years ago
Comment on attachment 108817 [details] [diff] [review]
Proposed fix, v2

r=cavin.
Attachment #108817 - Flags: review+

### Updated

17 years ago
No longer depends on: 118038

### Comment 23

17 years ago
I don't see a new version of mailComposeEditorOverlay.xul


### Comment 24

17 years ago
Posted patch New file for patch v2 (obsolete) — Splinter Review
I forget to include those 2 new files in the previous patch

### Updated

17 years ago
Attachment #108982 - Flags: review?(cavin)

### Comment 25

17 years ago
Comment on attachment 108817 [details] [diff] [review]
Proposed fix, v2

sr=sspitzer on this patch, once you address these issues:

1)

is this code only used by mail?

+	   // Include all Links. Let mail decide which one to send or not
nsCOMPtr<nsIDOMHTMLAnchorElement> anchor
(do_QueryInterface(content));
if (anchor)

also, a typo in the comment:

-// Include all Links. Let mail decide which one to send or not
+// Include all Links. Let mail decide which ones to send or not

2)

+		   nsCOMPtr<nsIPref> prefs(do_GetService(kPrefCID, &rv));
+		   if (NS_SUCCEEDED(rv) && prefs)

nsIPref iss deprecated and should not be used.
See nsIPrefService and nsIPrefBranch for the new implementations.

3)

4)

no need to set this to null.

+  nsCOMPtr<nsIURI> base = nsnull;

5)

you forgot "mail.compose" in the pref name.

also, the code isn't windows only, it's just that file:///// is windows only
(but what if I manually did that on linux or mac?)

you should indicate in comment in this .js file (and the code) that you rely on
the fact that network files on windows
are \\foo\bar, and file:///// as urls in mozilla.

+// Set this preference to true do tell mozilla to not attach the source of a
+// network file (file://///<network name>/<path>/<file name>). Windows only
+

6)

make sure to get someone on editor to review, as well.

### Comment 26

17 years ago
from an prior comment, this depends on #118038, right?
Depends on: 118038

### Comment 27

17 years ago
Comment on attachment 108982 [details] [diff] [review]
New file for patch v2

r=cavin.
Attachment #108982 - Flags: review?(cavin) → review+

### Comment 28

17 years ago
Comment on attachment 108982 [details] [diff] [review]
New file for patch v2

I didn't review the whole patch, because I had some  issues.

1)

>     function SetupMailOverlayHook()
>     {
>       {
>       }
>     }

why aren't you doing:

can you also explain what the timer is for?

can you screen shot what this UI looks like, so that we can get jglick to
comment?

2)

> <!ENTITY attachImageSource.label         "Attach the source of the image to the message">
> <!ENTITY attachImageSource.accesskey     "s">
>
> <!ENTITY attachLinkSource.label          "Attach the source of the link to the message">

I'm not sure if this is the best wording.  Maybe "Attach a copy" instead of
"Attach the source".
Attachment #108982 - Flags: superreview-
Attachment #108982 - Flags: review?(cavin)
Attachment #108982 - Flags: review+

### Comment 29

17 years ago
This does not depend on bug 118038 anymore as charley backup his change until
bug 118038 is fixed.

I have asked cmanske to review the editor part
No longer depends on: 118038

### Comment 30

17 years ago
Comment on attachment 108982 [details] [diff] [review]
New file for patch v2

removing the r= request
Attachment #108982 - Flags: review?(cavin)

### Comment 31

17 years ago
>addEventListener("load",OnLoadOverlay,true);

events on child documents (iframe, browser, editor).

### Comment 32

17 years ago
I will use addEventListener, that works perfectly with it too.
I use a timer to detect when the image/link url has changed. I could listen for
"change" or "keypress" event but that will miss case when the url get changed by
the "Choose File" button. Unfortunately, I could not easely detect when the
chosse file dialog get closed! Therefore I use a time like editor does to update
the image preview.

I'll post soon a new patch as well some dump screen for the UI change...

### Comment 33

17 years ago
Posted patch Proposed fix, v3 (obsolete) — Splinter Review
Use addEventListener instead of hijacking the onload handler.

### Updated

17 years ago
Attachment #108982 - Attachment is obsolete: true

### Comment 34

17 years ago
cc'ing sspitzer

17 years ago

### Comment 36

17 years ago
Jennifer, can you review the UI changes: I added a checkbox in both the image
and link properties dialog. The new checkbox is visible only when the dialog is
open from message compose.

Seth suggested to use "Attach a copy" instead of
"Attach the source".



### Comment 37

17 years ago
This is only for local file links correct? We don't want to be attaching source
attach a copy of the file to the message" be more accurate? ("For local images,
attach a copy of the image to the message").

Or, why not just do what the menu item states "Insert a Link", but provide a
disclaimer note that it might not work for local files. "Note: When creating
shared network. Otherwise, attach the local file instead."


### Comment 38

17 years ago
This new feature is for remote and local files. By default only the source of
local file is sent/attached but the user has now the possibility to change that.
In some case, is important to be able to send/attach the source of a remote
image/file when this one is not available for every recipient (maybe behind a
firewall). In other case, you don't need to attach the source of a fine on a
local server.

About Jennifer second solution, it could works for links but not for inline images!

### Comment 39

17 years ago
If the user wants to attach a file, webpage, image, etc., to a message they
should use the "Attach" feature. If they just want to insert a link, they
should use the Link feature (and we should only include the link). Why are we
trying to duplicate the Attach feature in the Insert Link dialog?

I'd recommend that Insert Link just insert a link and Attach be used to attach
files, webpages, images, etc. If desired, a Note as mentioned in c#37 could be
used to explain why a link might not work.

### Comment 40

17 years ago
I agree with jglick's suggestion so we can avoid making the link dialog more
complicated.


### Comment 41

17 years ago
The target milestone should be changed isn't it ?
we're on moz 1.3beta now

### Comment 42

16 years ago
*** Bug 162802 has been marked as a duplicate of this bug. ***

### Comment 43

15 years ago
*** Bug 220772 has been marked as a duplicate of this bug. ***

### Updated

15 years ago
Product: MailNews → Core

### Comment 44

14 years ago
What needs to happen on this bug for the fix to be considered for inclusion?
Patches were submitted and reviewed two years ago. Then discussion seems to have
stopped over the issue of whether Insert Link should attach a local file at all.

In the mean time, Windows users have no discoverable way (one workaround was
mentioned in the comments) for inserting a link to a network file. This is
something I want to do on a daily basis within our business. My boss doesn't
want me emailing him 50MB attachments, nor does he want a text description of
how to find it on the network - he wants a link he can click to quickly open the
file. I imagine this would be the case in many other corporate environments.

To me, the argument that an Insert Link dialog should insert a link, rather than
attaching a file, is a strong one. Inserting a link and attaching a file are
orthogonal tasks. The UI has matching orthogonal elements (attach button, drag
to attachments area vs. insert link, drag to compose area). The worst part about
this bug is that it does something counterintuitive and non-orthogonal by
attaching a file when you attempt to insert a link.

If a decision were made which way to move forward, I would be happy to be a part
of working on a patch.

Should we:

1) add hidden pref for power users (existing patch includes this pref) to link

2) add to UI (per existing patch), adding check box to Insert File dialog, for
when someone is sure they want to link to a network/local file rather than attach.

3) Make Insert Link / Attach truly orthogonal by making them do exactly what
they say. Deal with concerns over privacy & linking to local files by adding
"Are you really sure you know what you're doing" message box when someone adds a
link to a local/network file. Allow D&D to work as per user expectations (at
least on Windows platform): by default, D&D attaches file when dropped anywhere
on compose window per "copy" paradigm. When D&D with modifier key or button,

Scott

### Comment 45

14 years ago
I think this bug has simply been forgotten. Setting severity to normal and
tentatively asking for blocking new versions.
Severity: enhancement → normal
Flags: blocking1.8b2?
Flags: blocking-aviary1.1?
Target Milestone: mozilla1.3beta → ---

### Comment 46

14 years ago
(In reply to comment #44)
> My boss doesn't want me emailing him 50MB attachments, nor does he want a
> text description of how to find it on the network - he wants a link he can
> click to quickly open the file. I imagine this would be the case in many
> other corporate environments.

You can't put the file on an in-house webserver and send him an http: link?
That *is* the way to make linking work as desired.  A file: link with a
hardcoded path to a file on a LAN is not only tangled up with privacy issues,
it's also unreliable (if, for instance, the drive letter mapping is different
between sender and receiver).

### Comment 47

14 years ago
Putting the file on a local webserver, or any webserver, would of course make
linking work, since http:// linking isn't broken in composer. But that's besides
the point. Why should I copy a file to a webserver when it is already on our
file server? That's the main reason to have a central file server - shared file
access.

In our corporate network, common file server directories are mapped to
consistent drive letters, enforced by domain policy, so that company files have
a consistent file path. This has been the case in every company I remember
working for, in fact. For corporate users, it makes complete sense to want to
send a file link to a co-worker, enough so that the absence of this ability in
Thunderbird is a real pain.

### Comment 48

14 years ago
it would be great to have a fix for this.  I've recently rolled out
Thunderbird/Firefox to 150 users and this is the one problem that we are having
as an organisation with the software.  We previously used Netscape 4.7 and staff
are used to sending links to policy documents etc on the network in internal
emails to save disk space rather than sending files as attachments (file links
worked OK in Netscape and we spent some time educating users to send links
rather than attachments so they are a bit bemused now that this function doesn't
work).

As with other comments on this bug, we use consistent drive mapping letters on
domain logon so there are consistent file paths to organisationl documents.  I
also agree with other comments that having a UI option to insert a link that
then inserts an attachment is confusing and un-intuitive.

At the very least, implementing a fix that provides a pref that can be set for
users in corporate environments who expect the software to handle file links
would be great.

Please, please can someone fix this - it looks like a fix is nearly there, but
it's beyond me to delve in the code.  We have some money as an organisation to
spend on fixing this, so would even be prepared to pay some to implement this as
an extension that we could install.  Can anyone help? cheers, emma



### Updated

14 years ago
Flags: blocking1.8b2? → blocking1.8b2-

### Updated

14 years ago
Flags: blocking-aviary1.1? → blocking1.8b4?
Whiteboard: have fix

### Comment 49

14 years ago
taking - I'll try to drive this in.
Assignee: ducarroz → bienvenu
Status: ASSIGNED → NEW

### Comment 50

14 years ago
Comment on attachment 109439 [details] [diff] [review]
Proposed fix, v3

Daniel, while I try to get this patch working, I wanted to check that you'd be
OK with the editor parts of it (if not, I'd need to find an other approach)
Attachment #109439 - Flags: review?(daniel)

### Comment 51

14 years ago
we'll certainly consider a reviewd patch but we're not going to block on this
unless someone makes a compelling case. 
Flags: blocking1.8b4? → blocking1.8b4-
Assignee

### Comment 52

14 years ago
Changes since v3.0
* Made corrections suggested (I think I captured them all anyway)
* Changed from using nsCaseInsensitiveStringComparator to using
LowerCaseEqualsLiteral
* Other bitrotting so it applies

### Comment 53

14 years ago
I've been trying to get this to work in Thunderbird. I've also un-bitrotted the
patch, and cleaned it up a little, especially redoing the sense of the pref,
which makes the code a little clearer. But I'm having trouble getting the
overlay to work in tbird.

### Comment 54

14 years ago
mscott's going to look at getting the overlay stuff working in tbird.
Assignee: bienvenu → mscott

### Comment 55

14 years ago
(In reply to comment #51)
> we'll certainly consider a reviewd patch but we're not going to block on this
> unless someone makes a compelling case.
Trying to make a compelling case:
There is no point providing an option in the UI to "create a link" to a file if
all it is doing is inserting some text and then attaching the file.
When evaluating the software for use I should have tested this feature more
thoroughly and found that it didn't quite do what it said on the box, BUT the
option shouldn't really be in the UI. Sending links to files is pretty standard
practise on corporate LANs with common drive mountings and this was a feature
that Netscape Mail provided.

### Comment 56

14 years ago
What's happening with this? Please can someone try to finish this patch?

We are having huge problems in our organisation due to our regional users
time they log on to Thunderbird. Everybody in our organisation has been taught
to send links to files rather than attachments in order to prevent this problem
from happening, but since we moved to Thunderbird we may as well be sending
attachments.

We cannot copy our documents to a web server as this is a hugely inefficient
use of disk space. The only thing we can afford to change is the email client
we use, and I know for a fact that other email clients provide the
functionality we are looking for. I don't want to stop using Thunderbird but I
don't see how Mozilla can appeal to organisations if this fix isn't
implemented.

For a further plea: We are an environmental charity and this is really
preventing us doing our jobs as efficiently as we could be. A fix for this
would be so greatly appreciated!

Thanks

Rich

### Comment 57

14 years ago
Asking for Seamonkey checkin as overlay problems seem to affect Thunderbird
only. Adding privacy keyword as by my comment #5. (Yes, it is quite usual to
help someone finding her/his system files by adding a file:/// link to a mail.)
Flags: blocking-seamonkey1.0b?
Flags: blocking-seamonkey1.0a?

### Comment 58

14 years ago
SeaMonkey 1.0 Alpha has facutally already happened, even if it's not announced
yet, so that chance is gone anyways. Also minusing for beta as it's just
nice-to-havem but I don't think we would hold the release for it. This doesn't
mean it can't go in, it just means we won't push out the release for this fix.
Flags: blocking-seamonkey1.0b?
Flags: blocking-seamonkey1.0b-
Flags: blocking-seamonkey1.0a?
Flags: blocking-seamonkey1.0a-

### Comment 59

14 years ago
*** Bug 315868 has been marked as a duplicate of this bug. ***

### Comment 60

14 years ago
*** Bug 324965 has been marked as a duplicate of this bug. ***

### Updated

14 years ago
Flags: blocking-thunderbird2?

### Comment 61

14 years ago
*** Bug 327857 has been marked as a duplicate of this bug. ***

### Comment 62

13 years ago
Having faith that the proposed v3 fix will get checked in soon for Corporate users, even though this bug is four years old, i'm adding myself to the cc list.

### Comment 63

13 years ago
Would it be possible to remove "local network" from the bug summary, since it's the only thing which prevents the bug 176416 from beeing marked as dupe of this one ?
Reporter

### Updated

13 years ago
Summary: Inserting a link to a local network file in a message insert the physical file ! → Inserting a link to a network file in a message insert the physical file !

### Comment 64

13 years ago
*** Bug 176416 has been marked as a duplicate of this bug. ***

### Updated

13 years ago
Summary: Inserting a link to a network file in a message insert the physical file ! → Inserting a link to a [network][image] file into a message inserts the physical file!

### Comment 65

13 years ago
David, perhaps should you consider requesting review from ducarroz or mscott.

### Updated

13 years ago
Attachment #109439 - Attachment is obsolete: true
Attachment #109439 - Flags: review?(daniel)

### Comment 66

13 years ago
Comment on attachment 190085 [details] [diff] [review]
Unbitrotted version of patch v3.0a

Daniel, thoughts on the editor part of this patch, i.e., nsHTMLEditor.cpp? Does code other than mailnews use this code?
Attachment #190085 - Flags: review?(daniel)

### Comment 67

13 years ago
I also posted this to 176416, which is close to a clone of this issue.

This should be converted to a request for a interface feature.
I'd like to see it in the T-bird  Tools -> Options -> Composition -> General dialog.
It could be a checkbox that defaults to true as:
[x] Embed images referenced by URL in outgoing message
If we un-check it, the image is not sent out embedded in the message, only the
URL.

I wonder what the odds are of this attracting the attention of someone who can actually change the code?

### Comment 68

13 years ago
This isn't really an issue of whether to show images from their URLs. This is about file URLs, and whether the mail app should honor the user's intent (insert a file link) or override the user's intent (load the file when the user clearly inserted a URL instead of an attachment).

And it appears to me that people have already submitted patches to fix this behavior. Are these changes going ignored because of a philosophical concern (that we shouldn't honor the user's request for some reason) or for administrative reasons?

This bug causes me no end of trouble on my corporate network. When I forget about this bug, I find Thunderbird attaching a 1.5MB file against my wishes, when I was clearly trying to insert a link to a file on the network that all of my recipients could easily have reached. This slows down the mail, making it painful for users connecting over our VPN, and causes unnecessary wasted resources on the mail server.

Please, can someone review the patches that have been submitted or, if they're bitrotted at this point (no surprise if they are), ask the submitter to update them?

### Comment 69

13 years ago
Pinging Daniel about the editor part of that patch...

### Comment 70

13 years ago
Comment on attachment 190085 [details] [diff] [review]
Unbitrotted version of patch v3.0a

r=daniel@glazman.org for editor part
Attachment #190085 - Flags: review?(daniel) → review+
Assignee

### Comment 71

13 years ago
Posted patch new unbitrotted version v3.1 (obsolete) — Splinter Review
Changes since v3.0a:
* Unbitrotted patch
* forceToBeAttached set to PR_TRUE instead of testing attributeValue against false as that would always be true anyway.
* necessary TB files altered to make use of the backend changes
Assignee: mscott → iann_bugzilla
Attachment #190085 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #235600 - Flags: review?(neil)

### Comment 72

13 years ago
Comment on attachment 235600 [details] [diff] [review]
new unbitrotted version v3.1

>         // See if it's an image or an embed
>         if (tagName.EqualsLiteral("img") || tagName.EqualsLiteral("embed"))
>           (*aNodeList)->AppendElement(node);
>         else if (tagName.EqualsLiteral("a"))
>         {
>+          // Include all Links. Let mail decide which one to send or not
>           nsCOMPtr<nsIDOMHTMLAnchorElement> anchor (do_QueryInterface(content));
>           if (anchor)
>-          {
>-            nsAutoString href;
>-            if (NS_SUCCEEDED(anchor->GetHref(href)))
>-              if (StringBeginsWith(href, NS_LITERAL_STRING("file:"),
>-                                   nsCaseInsensitiveStringComparator()))
>-                (*aNodeList)->AppendElement(node);
>-          }
>+            (*aNodeList)->AppendElement(node);
>         }
The AnchorElement QI only existed to get its href, you should be find merging the link test into the previous block.

>+      clearInterval(gMsgCompTimerID); //stop the timer to prevent conflicting we us
That shouldn't happen, the timer won't fire simultaneously.

It might be possible to replace SetRelativeCheckbox with this function,
that way we could be sure that it was called on any url change.

>+      // Does the URL has changed
"Has the URL changed"

>+            var url = /\s*(\S*):\/\/(\S*)/;
>+            var results = inputElement.value.match(url);
>+            if (results && results[1].toLowerCase() == "file")
>+            {
>+              //is it a Windows remote file?
>+              if (results[2].substr(0, 3) == "///")
Can't you do that all in one regexp test i.e.
if (^\s*file:\/\/\/\/\//i.test(url))

>+        gMsgCompAttachSourceElement.checked = (gMsgCompPrevMozDoNotSendAttribute == null || gMsgCompPrevMozDoNotSendAttribute != "true");
Hmm, that's not really accurate, is it? If it's null then it depends on the pref. Maybe you should test for the attribute change first, and if it was null then pretend the value changed and you need to reset the checkbox.

>+      var windowtype = window.opener.document.firstChild.getAttribute("windowtype");
Note: Other editor dialogs use GetCurrentEditor().flags & nsIPlaintextEditor.eEditorMailMask;

>+          switch (document.firstChild.getAttribute("id"))
document.documentElement.id

>+          gMsgCompAttachSourceElement.hidden = false;
Surely this should be false by default anyway?

This should be in the main script block, and not using capturing.

>+<!ENTITY attachImageSource.label         "Attach the source of the image to the message">
Shouldn't this say "Attach this image to the message"?

>+                if (!nsCRT::strncasecmp(urlSpec.get(), "file://///", 10))
You should use StringBeginsWith here.

>+  PRBool        mozDoNotSendIsFalse;
I don't understand the changes related to this member variable.
Attachment #235600 - Flags: review?(neil) → review-
Assignee

### Comment 73

13 years ago
Posted patch Revised patch v3.2 (obsolete) — Splinter Review
Changes since v3.1:
* Removed unneeded AnchorElement QI.
* Stopped using timer to test for input, replaces SetRelativeCheckbox function instead for mailnews editors as that function is not used for mailnews.
* Streamlined tests for networked files as suggested.
* Test for attribute change first and use the same code for both that and URL change to reset checkbox.
* Move addEventListener to main script block and stopped using capture in it.
* Tweaked entity label for images.
* Removed unneeded changes from nsMsgComposeAndSend::ProcessMultipartRelated.
Attachment #235600 - Attachment is obsolete: true
Attachment #235965 - Flags: review?(neil)

### Comment 74

13 years ago
Comment on attachment 235965 [details] [diff] [review]
Revised patch v3.2

Nice :-)

>+          // the rule should be in sync with to the one in nsMsgComposeAndSend::GetEmbeddedObjectInfo
Except I don't think it's quite right. The rule in GetEmbeddedObjectInfo appears to be correct, as follows:
If the image/link isn't a file, then attach an image but not a link.
If the file does not exist, then don't try to attach it.
Otherwise a link to a network file follows the pref.

I'm not too bothered if you don't check that the file exists, in which case your code is correct for images, but for links, you should try to attach a local file, unless it appears to be on the local network. Marking r- because I want to test out your next patch to see if it matches my expectations ;-)

>+                if (!StringBeginsWith(urlSpec, NS_LITERAL_CSTRING("file://///")))
I think your ! is spurious.
Attachment #235965 - Flags: review?(neil) → review-
Assignee

### Comment 75

13 years ago
Posted patch The no exclamation patch v3.3 (obsolete) — Splinter Review
Changes since v3.2:
* Got rid of the spurious !
* Renamed value to attach.
* If the "file" is not a network one, default attach to true.
Attachment #235965 - Attachment is obsolete: true
Attachment #236630 - Flags: review?(neil)

### Comment 76

13 years ago
Comment on attachment 236630 [details] [diff] [review]
The no exclamation patch v3.3

No, this isn't working right either. You're now defaulting to attaching all links, even to web pages, instead of just local files.
Attachment #236630 - Flags: review?(neil) → review+
Assignee

### Comment 77

13 years ago
Changes since v3.3:
* Checks link, defaults are: if network file - look at pref, if non-network file - attach, if not file at all - don't attach
Attachment #236630 - Attachment is obsolete: true
Attachment #236657 - Flags: review?(neil)

### Comment 78

13 years ago
Comment on attachment 236657 [details] [diff] [review]
Check for local too patch v3.4

>         label   = "&noAltText.label;"
>         accesskey = "&noAltText.accessKey;"
>         oncommand = "SetAltTextDisabled(true);"/>
>+    <!-- mail compose will insert custom item here defined in mailComposeEditorOverlay.xul -->
>   </vbox>
I've just realised that this isn't where the "URL is relative to page location" checkbox is. Is there any way you can put it there instead?
Attachment #236657 - Flags: review?(neil) → review+
Assignee

### Comment 79

13 years ago
Changes since v3.4
* Moved the attach to mail checkbox on the insert image dialog to the same place as the make relative checkbox.

Carrying forward r+ from daniel and neil and requesting sr.
Attachment #236657 - Attachment is obsolete: true
Attachment #236890 - Flags: superreview?(bienvenu)
Attachment #236890 - Flags: review+

### Comment 80

13 years ago
Comment on attachment 236890 [details] [diff] [review]
Checkbox by MakeRelative patch v3.5 (Checked into trunk)

+<!ENTITY attachImageSource.label         "Attach this image to the message">
+<!ENTITY attachImageSource.accesskey     "s">
+

should these have the same accessKey?

### Comment 81

13 years ago
Comment on attachment 236890 [details] [diff] [review]
Checkbox by MakeRelative patch v3.5 (Checked into trunk)

sr=bienvenu, modulo the question about the access keys being the same.
Attachment #236890 - Flags: superreview?(bienvenu) → superreview+
Assignee

### Comment 82

13 years ago
(In reply to comment #80)
> (From update of attachment 236890 [details] [diff] [review] )
> +<!ENTITY attachImageSource.label         "Attach this image to the message">
> +<!ENTITY attachImageSource.accesskey     "s">
> +
> the message">
>
>
> should these have the same accessKey?
>
They can be the same because they are for overlays to two different dialog boxes.
Assignee

### Comment 83

13 years ago
Comment on attachment 236890 [details] [diff] [review]
Checkbox by MakeRelative patch v3.5 (Checked into trunk)

Checking in (trunk)
editor/libeditor/html/nsHTMLEditor.cpp;
new revision: 1.544; previous revision: 1.543
editor/ui/dialogs/content/EdImageOverlay.xul;
new revision: 1.11; previous revision: 1.10
new revision: 1.82; previous revision: 1.81
mail/components/compose/jar.mn;
new revision: 1.21; previous revision: 1.20
mail/locales/jar.mn;
new revision: 1.42; previous revision: 1.41
mail/locales/en-US/chrome/messenger/messengercompose/mailComposeEditorOverlay.dtd;
initial revision: 1.1
mailnews/jar.mn;
new revision: 1.113; previous revision: 1.112
mailnews/mailnews.js;
new revision: 3.275; previous revision: 3.274
mailnews/base/resources/content/contents.rdf;
new revision: 1.41; previous revision: 1.40
mailnews/compose/src/nsMsgSend.cpp;
new revision: 1.382; previous revision: 1.381
mailnews/compose/resources/content/mailComposeEditorOverlay.xul;
initial revision: 1.1
mailnews/compose/resources/locale/en-US/mailComposeEditorOverlay.dtd;
initial revision: 1.1
done
Attachment #236890 - Attachment description: Checkbox by MakeRelative patch v3.5 → Checkbox by MakeRelative patch v3.5 (Checked into trunk)
Assignee

### Comment 84

13 years ago
Comment on attachment 236890 [details] [diff] [review]
Checkbox by MakeRelative patch v3.5 (Checked into trunk)

Asking for approval to go into 1.8.1 branch, might need to make for a few days first though.
Attachment #236890 - Flags: approval1.8.1?
Attachment #236890 - Flags: approval-thunderbird2?
Assignee

### Updated

13 years ago
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED

### Comment 85

13 years ago
Does this affect both FF and TBird - it is a large patch so we are concerned about risk this close (within a week) of RC1.

Assignee

### Comment 86

13 years ago
(In reply to comment #85)
> Does this affect both FF and TBird - it is a large patch so we are concerned
> about risk this close (within a week) of RC1.
>
Just TBird as FF does not make use of any of this code AFAIK


### Comment 87

13 years ago
Comment on attachment 236890 [details] [diff] [review]
Checkbox by MakeRelative patch v3.5 (Checked into trunk)

a=mconnor on behalf of drivers for the small non-mailnews code, looks like only mailnews code calls this, so it should be ok from our perspective.
Attachment #236890 - Flags: approval1.8.1? → approval1.8.1+

13 years ago
Blocks: 352317
Assignee

### Comment 88

13 years ago
This patch:
* Fixes the fact we only want forceToBeAttached to be true when moz_do_not_send exists and is set to false.
Attachment #238007 - Flags: superreview?(bienvenu)
Attachment #238007 - Flags: review?(neil)

### Updated

13 years ago
Attachment #238007 - Flags: superreview?(bienvenu) → superreview+

13 years ago
Blocks: 352341

### Updated

13 years ago
Attachment #238007 - Flags: review?(neil) → review+
Assignee

### Comment 89

13 years ago
Comment on attachment 238007 [details] [diff] [review]
forceToBeAttached fix (Checked into trunk)

Checking in (trunk)
nsMsgSend.cpp;
new revision: 1.383; previous revision: 1.382
done
Attachment #238007 - Attachment description: forceToBeAttached fix → forceToBeAttached fix (Checked into trunk)

### Comment 90

13 years ago
In testing this, it seems to be working pretty well (TB 3a1-0915).  Thanks, Ian; it's great to see a big old bug like this finally squashed.

One slight thing, which may not be a problem: if inserting an image, and I select the 'Attach' checkbox and then deselect it again, the message goes out with -moz-do-not-send=false in the <img> tag; but if I don't touch the checkbox at all, there is no -moz-do-not-send attribute at all.  Net result is the same, and it's not something that causes compatibility issues.

Good to know:  -moz-do-not-send=true  stays in place when forwarding-inline, so the image continues to remain remote.  However, if the message sender isn't in one's address book (or if whitelisting is turned off) the image will be blocked (good); but forward-inline of that image will automatically load the image in the compose window, ignoring the Block setting -- this is bug 263345.
Assignee

13 years ago
Blocks: 353086
Assignee

### Comment 91

13 years ago
This patch:
* Updates suite help to reflect changes implemented in this bug.
Attachment #238940 - Flags: review?(neil)

### Updated

13 years ago
Attachment #238940 - Flags: review?(neil) → review+

13 years ago
Blocks: 353186
Assignee

### Updated

13 years ago
No longer blocks: 353186
Assignee

### Comment 92

13 years ago
Comment on attachment 238940 [details] [diff] [review]
Help patch v3.5 (Checked into trunk)

Checking in (trunk)
composer_help.xhtml;
new revision: 1.34; previous revision: 1.33
done
Attachment #238940 - Attachment description: Help patch v3.5 → Help patch v3.5 (Checked into trunk)

### Comment 93

13 years ago
I think we should take this for 2.0, so we might as well get it before the beta to shake out any particular problems.

### Comment 94

13 years ago
David, I don't think we can take this for 2.0, it looks like it makes changes to editor which is code compiled by Firefox. We can no longer take changes that effect Firefox on the branch anymore. 

### Comment 95

13 years ago
the editor changes have approval 1.8.1, though I don't know if they were checked in, and if not, if it's too late to check them in...
Assignee

### Comment 96

13 years ago
(In reply to comment #94)
> David, I don't think we can take this for 2.0, it looks like it makes changes
> to editor which is code compiled by Firefox. We can no longer take changes that
> effect Firefox on the branch anymore.
>
The parts of editor being patched are not used by Firefox. GetEmbeddedObjects in nsHTMLEditor.cpp is only used by nsMsgCompose.cpp and nsMsgSend.cpp. So all that code is for TB/mailnews only, approval was asked for 1.8.1 as it does sit in core.
Assignee

### Comment 97

13 years ago
Comment on attachment 236890 [details] [diff] [review]
Checkbox by MakeRelative patch v3.5 (Checked into trunk)

Requesting a= for 1.8.1.1 - would like to get this into branch before TB2 but after FF2. This code is compiled for Firefox but not used by it, only by TB and SM.
Attachment #236890 - Flags: approval1.8.1.1?
Assignee

### Comment 98

13 years ago
Scott, I'd like to get the non-core string changes in now, can you a= that?
Then once FF2 is out get the core changes and other non-core changes in.

### Comment 99

13 years ago
That's a great idea Ian. Do you want to attach a patch that has just the locale changes and i'll approve that? Good thinking.
Assignee

### Comment 100

13 years ago
Locale only changes for branch approval/checkin.
Attachment #242118 - Flags: superreview+
Attachment #242118 - Flags: review+
Attachment #242118 - Flags: approval-thunderbird2?

### Updated

13 years ago
Attachment #242118 - Flags: approval-thunderbird2? → approval-thunderbird2+
Assignee

### Comment 101

13 years ago
Comment on attachment 242118 [details] [diff] [review]
Locale changes for branch only v0.1 (Checked into 1.8.1 branch)

Checking in (1.8.1 branch)
extensions/help/resources/locale/en-US/composer_help.xhtml;
new revision: 1.31.6.2; previous revision: 1.31.6.1
mail/locales/jar.mn;
new revision: 1.35.2.6; previous revision: 1.35.2.5
mail/locales/en-US/chrome/messenger/messengercompose/mailComposeEditorOverlay.dtd;
new revision: 1.1.4.1; previous revision: 1.1
mailnews/jar.mn;
new revision: 1.99.2.11; previous revision: 1.99.2.10
mailnews/compose/resources/locale/en-US/mailComposeEditorOverlay.dtd;
new revision: 1.1.4.1; previous revision: 1.1
done
Attachment #242118 - Attachment description: Locale changes for branch only v0.1 → Locale changes for branch only v0.1 (Checked into 1.8.1 branch)
Assignee

### Updated

13 years ago
Attachment #236890 - Flags: approval1.8.1.1?
Attachment #236890 - Flags: approval-thunderbird2?
Assignee

### Comment 102

13 years ago
Requesting a= for TB2, SM1.1b and 1.8.1.1 core, carrying forward r= and sr=
As mentioned before, some of the code these changes effect is compiled by Firefox but not actually used, it is only used by TB and SM.
Attachment #242121 - Flags: superreview+
Attachment #242121 - Flags: review+
Attachment #242121 - Flags: approval1.8.1.1?
Attachment #242121 - Flags: approval-thunderbird2?
Assignee

### Updated

13 years ago
Flags: blocking1.8.1.1?

### Comment 103

13 years ago
glazman or anyone else: Are we sure that the changes to nsHTMLEditor.cpp will not affect midas?

### Updated

13 years ago
Flags: blocking1.8.1.1? → blocking1.8.1.1-

### Comment 104

13 years ago
Comment on attachment 242121 [details] [diff] [review]
Non-locale changes (inc core) for branch patch v0.2 (Checked into 1.8.1 branch)

I talked this over with Dan who read the helpful e-mail Neil sent to drivers.
Attachment #242121 - Flags: approval1.8.1.1?
Attachment #242121 - Flags: approval1.8.1.1+
Attachment #242121 - Flags: approval-thunderbird2?
Attachment #242121 - Flags: approval-thunderbird2+

### Comment 105

13 years ago
Comment on attachment 242121 [details] [diff] [review]
Non-locale changes (inc core) for branch patch v0.2 (Checked into 1.8.1 branch)

a=me for sm1.1 (approval requested by iann)
Assignee

### Comment 106

13 years ago
Comment on attachment 242121 [details] [diff] [review]
Non-locale changes (inc core) for branch patch v0.2 (Checked into 1.8.1 branch)

Checking in (1.8.1 branch)
editor/libeditor/html/nsHTMLEditor.cpp;
new revision: 1.529.2.9; previous revision: 1.529.2.8
editor/ui/dialogs/content/EdImageOverlay.xul;
new revision: 1.10.28.1; previous revision: 1.10
new revision: 1.81.28.1; previous revision: 1.81
mail/components/compose/jar.mn;
new revision: 1.18.4.1; previous revision: 1.18
mailnews/jar.mn;
new revision: 1.99.2.15; previous revision: 1.99.2.14
mailnews/mailnews.js;
new revision: 3.249.2.29; previous revision: 3.249.2.28
mailnews/base/resources/content/contents.rdf;
new revision: 1.40.4.1; previous revision: 1.40
mailnews/compose/resources/content/mailComposeEditorOverlay.xul;
new revision: 1.1.4.1; previous revision: 1.1
mailnews/compose/src/nsMsgSend.cpp;
new revision: 1.374.2.5; previous revision: 1.374.2.4
done
Attachment #242121 - Attachment description: Non-locale changes (inc core) for branch patch v0.2 → Non-locale changes (inc core) for branch patch v0.2 (Checked into 1.8.1 branch)
Assignee

13 years ago

### Updated

13 years ago
Status: RESOLVED → VERIFIED

### Comment 107

13 years ago
verified non-existence of UI changes in 20061109 on Mac. Loaded latest update, verified UI additions in link properties and image link properties.
Keywords: verified1.8.1.1

### Comment 108

13 years ago
(In reply to comment #107)
> verified non-existence of UI changes in 20061109 on Mac. Loaded latest update,
>

should have included build id of 20061129 for verified version of Thunderbird.`

### Updated

13 years ago
Flags: blocking-thunderbird2?

### Updated

13 years ago
Duplicate of this bug: 366492

### Updated

13 years ago
Duplicate of this bug: 372435

### Updated

11 years ago
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.