Closed Bug 1457457 Opened 3 years ago Closed 3 years ago

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

Categories

(Thunderbird :: Instant Messaging, defect)

57 Branch
defect
Not set
normal

Tracking

(thunderbird_esr60 fixed, thunderbird60 fixed, thunderbird61 wontfix, thunderbird62 fixed)

RESOLVED FIXED
Thunderbird 62.0
Tracking Status
thunderbird_esr60 --- fixed
thunderbird60 --- fixed
thunderbird61 --- wontfix
thunderbird62 --- fixed

People

(Reporter: cardbook.thunderbird, Assigned: jorgk-bmo)

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

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
Why is this a TB bug? Something in TB doesn't work?
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...
ooops sorry for the noise... It is working on Linux with Thunderbird 52 (it is with Thunderbird 57 that it fails)
(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?
Not working with Thunderbird 60.0b5 on Linux either.
to summarize :

Thunderbird 52.7.0 : working Windows + Linux
Thunderbird 60.0b5 : working Windows, not working Linux
(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
Flags: needinfo?(richard.marti)
Flags: needinfo?(clokep)
Flags: needinfo?(acelists)
(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
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.
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...
There is also no decodeImage() function.
So yes, as chat uses that one, it needs to be updated too. Thanks.
Flags: needinfo?(acelists)
Do we know how chat is affected?
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.
Flags: needinfo?(richard.marti)
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.)
Flags: needinfo?(clokep)
Aceman, could you do a patch for us here.
Flags: needinfo?(acelists)
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);
	});
Should something like:
* imgTools.decodeImageData -- replacement: decodeImageFromArrayBuffer
be added to:
https://wiki.mozilla.org/Thunderbird/Add-ons_Guide_57#Changes_in_thunderbird57
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.
Actually, TB switched from decodeImageData() to decodeImage() here:
https://hg.mozilla.org/comm-central/rev/cc8153293e2e
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.
Status: UNCONFIRMED → NEW
Component: Untriaged → Instant Messaging
Depends on: 1387790
Ever confirmed: true
Keywords: regression
OS: Unspecified → All
Hardware: Unspecified → All
Summary: imgTools.decodeImageData is not a function on Linux → "imgITools.decodeImageData is not a function" due to its rename in bug 1387790
Version: 52 Branch → 57 Branch
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.
No longer depends on: 1387790
Flags: needinfo?(acelists)
Summary: "imgITools.decodeImageData is not a function" due to its rename in bug 1387790 → Port bug 1420223: Replace imgITools.decodeImage() with imgITools.decodeImageBuffer() - see comment #23
Attached patch 1457457-decodeImageBuffer.patch (obsolete) — Splinter Review
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.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8972684 - Flags: review?(clokep)
Attachment #8972684 - Flags: review?(acelists)
Attached patch 1457457-decodeImageBuffer.patch (obsolete) — Splinter Review
Removed trailing white-space. Sorry about the noise.
Attachment #8972684 - Attachment is obsolete: true
Attachment #8972684 - Flags: review?(clokep)
Attachment #8972684 - Flags: review?(acelists)
Attachment #8972685 - Flags: review?(clokep)
Attachment #8972685 - Flags: review?(acelists)
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.
Attachment #8972685 - Flags: review?(acelists) → feedback+
Well, M-C use that code.
Yes, I see. It's up to a chat peer to decide on that risk.
seems to me that the function decodeImageBuffer() was renamed to decodeImageFromBuffer()...
Summary: Port bug 1420223: Replace imgITools.decodeImage() with imgITools.decodeImageBuffer() - see comment #23 → Port bug 1420223 and bug 1421655 : Replace imgITools.decodeImage() with imgITools.decodeImageFromBuffer() - see comment #23
Catering for a further rename in bug 1421655 (part 3) :-(
Attachment #8972685 - Attachment is obsolete: true
Attachment #8972685 - Flags: review?(clokep)
Attachment #8972826 - Flags: review?(clokep)
Attachment #8972826 - Flags: review?(florian)
Attachment #8972826 - Flags: review?(mkmelin+mozilla)
Attachment #8972826 - Flags: review?(martin)
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/242d96a1e0de
Port bug 1420223 and bug 1421655: Replace imgITools.decodeImage() with imgITools.decodeImageFromBuffer(). rs=bustage-fix DONTBUILD
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Whiteboard: [PLR]
Target Milestone: --- → Thunderbird 62.0
Attachment #8972826 - Flags: review?(mkmelin+mozilla) → review+
Attachment #8972826 - Flags: review?(martin)
Attachment #8972826 - Flags: review?(florian)
Attachment #8972826 - Flags: review?(clokep)
Whiteboard: [PLR]
You need to log in before you can comment on or make changes to this bug.