1241647 - vcard using deprecated nsIFile as newChannel

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, defect)

Description

[Kent James (:rkent)]

While looking for an issue in ExQuilla, I came across a similar issue in mail code. nsIFile can no longer be passed to asyncFetch, but this code does that:

http://mxr.mozilla.org/comm-central/source/mail/components/nsMailDefaultHandler.js#370

367         // A VCard! Be smart and open the "add contact" dialog.
368         let file = cmdLine.resolveFile(uri);
369         if (file.exists() && file.fileSize > 0) {
370           NetUtil.asyncFetch(file, function(inputStream, status) {
371             if (!Components.isSuccessCode(status)) {
372               return;
373             }

I would expect this to generate an assertion in debug builds. This assertion was added circa gecko 42

Comment 1

[Magnus Melin [:mkmelin]]

Attachment: bug1241647_vcard_channel.patch

For testing, give some .vcf file as argument on the command line - that should open the add contact dialog.

This is one of the new supported ways
http://mxr.mozilla.org/comm-central/source/mozilla/netwerk/base/NetUtil.jsm#305

Comment 2

[:aceman]

Yes, in a debug build this produces an error:
Error: NS_ERROR_FILE_TARGET_DOES_NOT_EXIST: Component returned failure code: 0x80520006 (NS_ERROR_FILE_TARGET_DOES_NOT_EXIST) [nsICommandLine.resolveFile]
Source File: file:///var/SSD/TB-hg/tbird-bin/dist/bin/components/nsMailDefaultHandler.js
Line: 368

And even causes the main TB window to not open.

Comment 3

[:aceman]

Actually not, I really had the path to the file wrong.

Comment 4

[:aceman]

Am I supposed to see any difference with the patch? I do not see one. If I give correct file, it opens the add contact window (with or without patch). For some invalid paths, it just hangs silently. For some invalid paths it produces the error I pasted.

Or is this just using the newer api but the old one is still working?

Comment 5

[Kent James (:rkent)]

aceman: Are you using a debug build? I believe it is still supported in release builds.

Comment 6

[:aceman]

Yes, I tried the patch on trunk debug build.

Comment 7

[Magnus Melin [:mkmelin]]

At least in the NetUtil.jsm code it's just deprecated. The old way is still working so it's just about using the newer api.

Comment 8

[:aceman]

Comment on attachment 8711173 [details] [diff] [review]
bug1241647_vcard_channel.patch

Review of attachment 8711173 [details] [diff] [review]:
-----------------------------------------------------------------

OK then.

::: mail/components/nsMailDefaultHandler.js
@@ +367,5 @@
>          // A VCard! Be smart and open the "add contact" dialog.
>          let file = cmdLine.resolveFile(uri);
>          if (file.exists() && file.fileSize > 0) {
> +          let uriSpec = Services.io.newFileURI(file).spec;
> +          NetUtil.asyncFetch({uri: uriSpec, loadUsingSystemPrincipal: true}, function(inputStream, status) {

Can you break the line before the "function"? It seems quite long now.

Comment 9

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/comm-central/rev/a095c7afeb05 -> FIXED

No need to take this on branches, the old way still works.