1457457 - Port bug 1420223 and bug 1421655 : Replace imgITools.decodeImage() with imgITools.decodeImageFromBuffer() - see comment #23

Status: RESOLVED FIXED

(Thunderbird :: Instant Messaging, defect)

Description

[Philippe VIGNEAU]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0
Build ID: 20180323154952

Steps to reproduce:

var imagedata = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAoAAAAKCAYAAACNMs+9AAAABGdBTUEAALGPC/xhBQAAAAlwSFlzAAALEwAACxMBAJqcGAAAAAd0SU1FB9YGARc5KB0XV+IAAAAddEVYdENvbW1lbnQAQ3JlYXRlZCB3aXRoIFRoZSBHSU1Q72QlbgAAAF1JREFUGNO9zL0NglAAxPEfdLTs4BZM4DIO4C7OwQg2JoQ9LE1exdlYvBBeZ7jqch9//q1uH4TLzw4d6+ErXMMcXuHWxId3KOETnnXXV6MJpcq2MLaI97CER3N0vr4MkhoXe0rZigAAAABJRU5ErkJggg==';

var io         = Components.classes["@mozilla.org/network/io-service;1"].getService(Components.interfaces.nsIIOService);
var channel    = io.newChannel(imagedata, null, null);
var input      = channel.open();
var imgTools   = Components.classes["@mozilla.org/image/tools;1"].getService(Components.interfaces.imgITools);

var container  = {};
imgTools.decodeImageData(input, channel.contentType, container);

var wrapped = Components.classes["@mozilla.org/supports-interface-pointer;1"].createInstance(Components.interfaces.nsISupportsInterfacePointer);
wrapped.data = container.value;

var trans = Components.classes["@mozilla.org/widget/transferable;1"].createInstance(Components.interfaces.nsITransferable);
trans.addDataFlavor(channel.contentType);
trans.setTransferData(channel.contentType, wrapped, -1);

var clipid = Components.interfaces.nsIClipboard;
var clip   = Components.classes["@mozilla.org/widget/clipboard;1"].getService(clipid);
clip.setData(trans, null, clipid.kGlobalClipboard);


Actual results:

with Thunderbird 52, this code works on Windows but not on Linux... I'm getting : "imgTools.decodeImageData is not a function"


Expected results:

image copied to the clipboard

Comment 1

[Philippe VIGNEAU]

code taken here : https://bugzilla.mozilla.org/show_bug.cgi?format=default&id=750108

Comment 2

[Jorg K (CEST = GMT+2)]

Why is this a TB bug? Something in TB doesn't work?

Comment 3

[Philippe VIGNEAU]

Hi Jorg

there is no functional issue with Thunderbird, just for an issue for addon that want to use the code given here : https://bugzilla.mozilla.org/show_bug.cgi?format=default&id=750108...

I'd like to try to understand why the Components.interfaces.imgITools is different on Windows and on Linux...

Comment 4

[Philippe VIGNEAU]

ooops sorry for the noise... It is working on Linux with Thunderbird 52 (it is with Thunderbird 57 that it fails)

Comment 5

[Óvári]

(In reply to Philippe VIGNEAU from comment #4)
> ooops sorry for the noise... It is working on Linux with Thunderbird 52 (it
> is with Thunderbird 57 that it fails)

Not working with Thunderbird 60.0b4 on Linux. Does that mean there is a regression that need to be fixed?

Comment 6

[Óvári]

Not working with Thunderbird 60.0b5 on Linux either.

Comment 7

[Philippe VIGNEAU]

to summarize :

Thunderbird 52.7.0 : working Windows + Linux
Thunderbird 60.0b5 : working Windows, not working Linux

Comment 8

[Jorg K (CEST = GMT+2)]

(In reply to Óvári from comment #5)
> Not working with Thunderbird 60.0b4 on Linux. Does that mean there is a
> regression that need to be fixed?
Sure. But imgITools is an Mozilla Central component, so please file a bug there.

We use imgItools only in chat. Is there a mal-function in chat on Linux somewhere due to this?
https://dxr.mozilla.org/comm-central/search?q=imgtools&redirect=false

Comment 9

[Óvári]

(In reply to Jorg K (GMT+1) from comment #8)
> (In reply to Óvári from comment #5)
> > Not working with Thunderbird 60.0b4 on Linux. Does that mean there is a
> > regression that need to be fixed?
> Sure. But imgITools is an Mozilla Central component, so please file a bug
> there.
Does this relate to any of these:
Bug 527007 - imgITools::DecodeImageData(...) doesn't support unbuffered streams (such as nsIFileInputStream), must use nsIBufferedInputStream (in nsIBufferedStreams.h)
Bug 816362 - Refactor imgTools::DecodeImageData to remove imgIContainer argument and use ImageFactory
Bug 1172496 - crash in imgTools::GetImgCacheForDocument
Bug 1181863 - Add a function similar to imgTools::DecodeImage which works off-main-thread

Comment 10

[:aceman]

I see no decodeImageData() method in msgITools (https://dxr.mozilla.org/comm-central/source/mozilla/image/imgITools.idl).
It was removed in bug 1387790.
See the patch https://hg.mozilla.org/mozilla-central/rev/3ceb0bc8a730 for replacements.

Chat seems to only use functions that are still in the file.

Comment 11

[Philippe VIGNEAU]

If I use the code used in chat I get "imgTools.decodeImage is not a function"... so I think there also is a problem in the chat...

Comment 12

[:aceman]

There is also no decodeImage() function.
So yes, as chat uses that one, it needs to be updated too. Thanks.

Comment 13

[Jorg K (CEST = GMT+2)]

Do we know how chat is affected?

Comment 14

[Richard Marti (:Paenglab)]

I can't say if it affected. It seems it's used to show a user icon but I don't know which protocol uses this.

Comment 15

[Jorg K (CEST = GMT+2)]

XMPP/Jabber? - https://en.wikipedia.org/wiki/XMPP

Comment 16

[Patrick Cloke [:clokep]]

Yes, it's used by XMPP to encode the image for sending as part of your vCard. (When another user requests your information we send your contact image as part of it.)

Comment 17

[Jorg K (CEST = GMT+2)]

Aceman, could you do a patch for us here.

Comment 18

[Philippe VIGNEAU]

I finally have found a workaround using the decodeImageFromArrayBuffer() function. Here is my code to copy an image file to the clipboard as an image (works for Thunderbird 52 and 60) :

var imgTools = Components.classes["@mozilla.org/image/tools;1"].getService(Components.interfaces.imgITools);
var myFileURISpec = document.getElementById('photolocalURITextBox').value;
var myFileURI = Services.io.newURI(myFileURISpec, null, null);

// Thunderbird 52 and Linux
if (imgTools.decodeImageData) {
	var myExtension = cardbookUtils.getFileNameExtension(myFileURISpec);
	var imagedata = 'data:image/' + myExtension + ';base64,' + btoa(cardbookSynchronization.getFileBinary(myFileURI));
	var io = Components.classes["@mozilla.org/network/io-service;1"].getService(Components.interfaces.nsIIOService);
	var channel = io.newChannel(imagedata, null, null);
	var input = channel.open();
	var container = {};
	imgTools.decodeImageData(input, channel.contentType, container);
	var wrapped = Components.classes["@mozilla.org/supports-interface-pointer;1"].createInstance(Components.interfaces.nsISupportsInterfacePointer);
	wrapped.data = container.value;
	var trans = Components.classes["@mozilla.org/widget/transferable;1"].createInstance(Components.interfaces.nsITransferable);
	trans.addDataFlavor(channel.contentType);
	trans.setTransferData(channel.contentType, wrapped, -1);
	var clipid = Components.interfaces.nsIClipboard;
	var clipboard = Components.classes["@mozilla.org/widget/clipboard;1"].getService(clipid);
	clipboard.setData(trans, null, clipid.kGlobalClipboard);
// Thunderbird 60
} else if (imgTools.decodeImageFromArrayBuffer) {
	var myChannel = Services.io.newChannelFromURI2(myFileURI,
							 null,
							 Services.scriptSecurityManager.getSystemPrincipal(),
							 null,
						Components.interfaces.nsILoadInfo.SEC_REQUIRE_SAME_ORIGIN_DATA_INHERITS,
							 Components.interfaces.nsIContentPolicy.TYPE_OTHER);
	NetUtil.asyncFetch(myChannel, function (inputStream, status) {
		if (!Components.isSuccessCode(status)) {
			return;
		}
		var octetArray = [];
		var binaryIS = Components.classes["@mozilla.org/binaryinputstream;1"].createInstance(Components.interfaces.nsIBinaryInputStream);
		binaryIS.setInputStream(inputStream);
		octetArray = binaryIS.readByteArray(binaryIS.available());
		var arrayBuffer = (new Int8Array(octetArray)).buffer;
		var container = imgTools.decodeImageFromArrayBuffer(arrayBuffer, myChannel.contentType);
		var wrapped = Components.classes["@mozilla.org/supports-interface-pointer;1"].createInstance(Components.interfaces.nsISupportsInterfacePointer);
		wrapped.data = container;
		var trans = Components.classes["@mozilla.org/widget/transferable;1"].createInstance(Components.interfaces.nsITransferable);
		trans.addDataFlavor(myChannel.contentType);
		trans.setTransferData(myChannel.contentType, wrapped, -1);
		var clipid = Components.interfaces.nsIClipboard;
		var clipboard = Components.classes["@mozilla.org/widget/clipboard;1"].getService(clipid);
		clipboard.setData(trans, null, clipid.kGlobalClipboard);
	});

Comment 19

[Óvári]

Should something like:
* imgTools.decodeImageData -- replacement: decodeImageFromArrayBuffer
be added to:
https://wiki.mozilla.org/Thunderbird/Add-ons_Guide_57#Changes_in_thunderbird57

Comment 20

[Jorg K (CEST = GMT+2)]

We'll add something to the guide. Right now I'm a little confused why the replacement function decodeImage()
https://hg.mozilla.org/mozilla-central/rev/3ceb0bc8a730#l4.9
wouldn't be working.

Aceman is on Linux, so I hope he can look at this soon.

Comment 21

[Jorg K (CEST = GMT+2)]

Actually, TB switched from decodeImageData() to decodeImage() here:
https://hg.mozilla.org/comm-central/rev/cc8153293e2e

Comment 22

[:aceman]

Because decodeImage() was renamed too in https://hg.mozilla.org/mozilla-central/rev/e60f002f07d0.

And this is not Linux specific, the methods were simply renamed, globally.

Comment 23

[Jorg K (CEST = GMT+2)]

This has become far too confusing. These are the facts:

We originally used decodeImageData(). That was replaced by decodeImage() in bug 1387790 which we followed in bug 1389475 in August 2017.

We didn't notice that decodeImage() was also replaced by decodeImageBuffer() in bug 1420223 in November 2017 here:
https://hg.mozilla.org/mozilla-central/rev/e60f002f07d0#l4.29

So we simply need to port 1420223.

As noted in comment #18, decodeImageFromArrayBuffer() is also a function that could be used.

Comment 24

[Jorg K (CEST = GMT+2)]

Attachment: 1457457-decodeImageBuffer.patch

Straight port of:
https://hg.mozilla.org/mozilla-central/rev/e60f002f07d0#l6.90
-container = imgTools.decodeImage(istream, inMimeType);
+buffer = NetUtil.readInputStreamToString(istream, istream.available());
+container = imgTools.decodeImageBuffer(buffer, buffer.length, inMimeType);

One review will do, whoever comes first. This should be corrected in TB 60 asap.

Comment 25

[Jorg K (CEST = GMT+2)]

Attachment: 1457457-decodeImageBuffer.patch

Removed trailing white-space. Sorry about the noise.

Comment 26

[:aceman]

Comment on attachment 8972685 [details] [diff] [review]
1457457-decodeImageBuffer.patch

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

I can't test XMPP so I'll let clokep test this, but I do not trust stream.available(), see bug 1356780. Please use a read loop for safety.

Comment 27

[Jorg K (CEST = GMT+2)]

Well, M-C use that code.

Comment 28

[:aceman]

Yes, I see. It's up to a chat peer to decide on that risk.

Comment 29

[Philippe VIGNEAU]

seems to me that the function decodeImageBuffer() was renamed to decodeImageFromBuffer()...

Comment 30

[Jorg K (CEST = GMT+2)]

Yes, here:
https://hg.mozilla.org/mozilla-central/rev/75d2ae4d07c4#l2.28

Comment 31

[Jorg K (CEST = GMT+2)]

Attachment: 1457457-decodeImageBuffer.patch

Catering for a further rename in bug 1421655 (part 3) :-(

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/242d96a1e0de
Port bug 1420223 and bug 1421655: Replace imgITools.decodeImage() with imgITools.decodeImageFromBuffer(). rs=bustage-fix DONTBUILD

Comment 33

[Jorg K (CEST = GMT+2)]

TB 60 beta 8 (BETA_60_CONTINUATION branch):
https://hg.mozilla.org/releases/comm-beta/rev/21436230ba7f5fd8f7f6d59a3c48a8dba5d4b844

Comment 34

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/releases/comm-esr60/rev/fe9e70da4e7b