jeremy-compostella / org-msg

OrgMsg is a GNU/Emacs global minor mode mixing up Org mode and Message mode to compose and reply to emails in a Outlook HTML friendly style.
GNU General Public License v3.0
276 stars 57 forks source link

MML expansion is skipped on Windows #122

Open morganwillcock opened 3 years ago

morganwillcock commented 3 years ago

I'm pretty sure that this is an issue within mml-expand-html-into-multipart-related, but it seems that the strings representing Windows path specs aren't supported by the MML expansion process and so OrgMsg ends up silently dropping content when it sends messages.

I initially thought the issue was that OrgMsg was removing the protocol specification for image source paths too soon, but that doesn't seem to make a difference in how the filename is returned from the URL parser:

> (url-generic-parse-url "file:///d:/one/two/three.png")
#s(url :type "file" :user nil :password nil :host "" :portspec nil :filename "/d:/one/two/three.png" :target nil :attributes nil :fullness t :silent nil :use-cookies t :asynchronous t)

> (url-filename (url-generic-parse-url "file:///d:/one/two/three.png"))
"/d:/one/two/three.png"

> (url-generic-parse-url "/d:/one/two/three.png")
#s(url :type nil :user nil :password nil :host nil :portspec nil :filename "/d:/one/two/three.png" :target nil :attributes nil :fullness nil :silent nil :use-cookies t :asynchronous t)

> (url-filename (url-generic-parse-url "/d:/one/two/three.png"))
"/d:/one/two/three.png"

The issue seems to be within mml-expand-html-into-multipart-related when it checks that the file system path exists based on the URL returned and skips processing otherwise:

(file-exists-p (url-filename parsed))

This always fails because the URL path returned isn't valid as a file system path and so the MML expansion is always skipped for any expanded Windows path spec:

> (file-exists-p "/d:/one/two/three.png")
nil

> (file-exists-p "c:/Windows/explorer.exe")
t

> (file-exists-p "/c:/Windows/explorer.exe")
nil

So at this point I'm presuming that the mml-expand-html-into-multipart-related function doesn't work on Windows and therefore OrgMsg isn't going to work well either. Or have I missed something?

Thanks, Morgan

jeremy-compostella commented 3 years ago

Seems to a reasonable investigation. Any chance you "fix" the path in the img tags before it get passed to the mml-expand-html-into-multipart-related ?

morganwillcock commented 3 years ago

It doesn't seem easy. It is possible to rewrite the string and send it through as "file:d:/one/two/three.png". This gives the correct result in terms of the filesystem path but also changes the URL type:

> (setq myurl (url-generic-parse-url "file:d:/one/two/three.png"))
#s(url :type "file" :user nil :password nil :host nil :portspec nil :filename "d:/one/two/three.png" :target nil :attributes nil :fullness nil :silent nil :use-cookies t :asynchronous t)

> (url-filename myurl)
"d:/one/two/three.png"
> (url-type myurl)
"file"

...and unfortunately the checks in mml-expand-html-into-multipart-related are looking for the type to be nil:

(when (and (null (url-type parsed))
           (url-filename parsed)
           (file-exists-p (url-filename parsed)))
  ...

Possibly that type check is not required since traditional paths that include the protocol still return the correct filesystem path:

> (setq myurl (url-generic-parse-url "file:///one/two/three.png"))
#s(url :type "file" :user nil :password nil :host "" :portspec nil :filename "/one/two/three.png" :target nil :attributes nil :fullness t :silent nil :use-cookies t :asynchronous t)

> (url-filename myurl)
"/one/two/three.png"
> (url-type myurl)
"file"
jeremy-compostella commented 3 years ago

What about the following hack?

modified   org-msg.el
@@ -1023,6 +1023,11 @@ If FILE does not have an extension, \"text/plain\" is returned."
       (mailcap-extension-to-mime extension)
     "text/plain"))

+(defun my-url-generic-parse-url-filter (url)
+  (when (string= (url-type url) "file")
+    (setf (url-type url) nil)
+    (setf (url-filename url) (substring (url-filename url) 1))))
+
 (defun org-msg-mml-into-multipart-related (orig-fun cont)
   "Extend the capability to handle file attachments.
 This function is used as an advice function of
@@ -1031,7 +1036,10 @@ This function is used as an advice function of
 - CONT is the MIME representation of the mail content.
 The implementation depends on the `org-msg-attachment' temporary
 variable set by `org-msg-prepare-to-send'."
+  (advice-add 'url-generic-parse-url
+         :filter-return 'my-url-generic-parse-url-filter)
   (setq cont (funcall orig-fun cont))
+  (advice-remove 'url-generic-parse-url #'my-url-generic-parse-url-filter)
   (let ((newparts '()))
     ;; Generate this list of attachment parts
     (dolist (file org-msg-attachment)
morganwillcock commented 3 years ago

I've played around with it a bit, eventually ending up with just this:

diff --git a/org-msg.el b/org-msg.el
index 1964589..7734a80 100644
--- a/org-msg.el
+++ b/org-msg.el
@@ -1023,6 +1023,14 @@ If FILE does not have an extension, \"text/plain\" is returned."
       (mailcap-extension-to-mime extension)
     "text/plain"))

+(defun org-msg-url-generic-parse-url-filter (url)
+  ;; When running on native Windows remove the leading slash from the filename
+  ;; if the path looks to be an absolute with a drive specification
+  (when (eq system-type 'windows-nt)
+    (setf (url-filename url) (replace-regexp-in-string
+                             "^/\\([A-Za-z]:\\/\\)" "\\1" (url-filename url))))
+  url)
+
 (defun org-msg-mml-into-multipart-related (orig-fun cont)
   "Extend the capability to handle file attachments.
 This function is used as an advice function of
@@ -1031,7 +1039,10 @@ This function is used as an advice function of
 - CONT is the MIME representation of the mail content.
 The implementation depends on the `org-msg-attachment' temporary
 variable set by `org-msg-prepare-to-send'."
+  (advice-add 'url-generic-parse-url
+             :filter-return 'org-msg-url-generic-parse-url-filter)
   (setq cont (funcall orig-fun cont))
+  (advice-remove 'url-generic-parse-url #'org-msg-url-generic-parse-url-filter)
   (let ((newparts '()))
     ;; Generate this list of attachment parts
     (dolist (file org-msg-attachment)

There looked to be a few problems:

I'll try to investigate further when I have time.