1206030 - Remove nsIDOMHTMLCanvasElement::MozFetchAsStream()

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

I introduced this a long time ago when working on the thumbnail service for the newtab page. The thumbnail service doesn't use it anymore and there are no add-ons out there using it either, let's get rid of it.

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1206030-Remove-nsIDOMHTMLCanvasElement-mozFetchA.patch

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 8662884 [details] [diff] [review]
0001-Bug-1206030-Remove-nsIDOMHTMLCanvasElement-mozFetchA.patch

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

I like, but you'll need a peer.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

(In reply to :Ms2ger from comment #2) 
> I like, but you'll need a peer.

Thought you were a peer, sorry :) Thanks for forwarding!

Comment 4

[Johnny Stenback (:jst)]

Comment on attachment 8662884 [details] [diff] [review]
0001-Bug-1206030-Remove-nsIDOMHTMLCanvasElement-mozFetchA.patch

I like removal of code we no longer need! r=jst

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/045b1309490f

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/045b1309490f

Comment 7

[Florian Scholz (Open Web Docs)]

https://developer.mozilla.org/en-US/Firefox/Releases/43#Changes_for_add-on_and_Mozilla_developers
https://developer.mozilla.org/en-US/docs/Web/API/HTMLCanvasElement/mozFetchAsStream

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

Thanks Florian!

Comment 9

[noitidart]

Was the justification to remove this purely that other addons dont use it? :(

I use it I use it in my addon :(

Its amazing. Because my goal is to post canvas data to a new tab. I do it here in Nativeshot I use it: https://github.com/Noitidart/NativeShot/blob/master/bootstrap.js#L1677

I feed the stream directly to nsIMultiplexStream: https://github.com/Noitidart/NativeShot/blob/master/bootstrap.js#L4528



The alternative is not as speedy or memory friendly. I would have to take it to blob. Then use file reader to convert to arraybuffer. then use nsIArrayBufferInputStream. I tried this approach but when arraybuffer is bigger then 64k it messes it up somehow.


Is there a chance we can still keep this?

Comment 10

[noitidart]

This stackoverflow topic explains what i mean by "post to tab":

http://stackoverflow.com/questions/25009431/replication-of-form-method-with-loadonetab/25020668#25020668

And here is a copy paste example just change the file path here to an image on your disk: https://github.com/Noitidart/_scratchpad/blob/5bcdb8d2c8358e2c2656b601ae0e960cc34c190b/upload%20image%20search%20to%20google.js#L65

And then copy paste that to scratchpad. It will open a new tab with the search results of matching images.

Comment 11

[noitidart]

The scratchpad copy paste example uses the very less performant nsIFile method. In my production i use mozFetchAsStream which is much more performant then the nsiFile method. And nsIFile will be deprecated eventually for sure, and it makes sense, as OS.File is fully async and very safe.

Comment 12

[noitidart]

I know my addon only has 10 users but thats because it hasn't been approved yet. My addon is going to replace LightShot addon which has 50k users, but that's Windows only. Mine also supports Linux and OS X so I'll easily have over 50k.

Here's a screencast showing how I use mozFetchAsStream to send data from canvas to loadOneTab as postData: https://www.youtube.com/watch?v=PhR0UWEwUvA

mozFetchAsStream is the fastest best way. And for postData to loadOneTab or XHR its just exceptional.

Comment 13

[Geoff Lankow (:darktrojan)]

(In reply to Tim Taubert [:ttaubert] from comment #0)
> there are no add-ons out there using it either

Not sure how you checked that but one of my add-ons used it too.

Comment 14

[Tim Taubert [:ttaubert] (inactive)]

Looks like I might have missed a few. Not sure how often we update the add-on MXR.

Comment 15

[noitidart]

Thanks @Geoff for chiming in.

Thanks so much @Tim if you can help bring this back. Could you tell us the chances of that please?

Do you know if my tests of "mozFetchAsStream direct to nsIMultiplexStream" being more performant then "canvas read to blob, then reading as array buffer, then putting into nsIArrayBufferInputStream, then finally adding to nsIMultiplexStream" are true? And do you expect it to be by a lot?

Comment 16

[noitidart]

(In reply to Tim Taubert [:ttaubert] from comment #14)
> Looks like I might have missed a few. Not sure how often we update the
> add-on MXR.

Hi there Tim, may you please update us on if there is chance we can get this back. My beta channel just updated to Firefox 43 and mozFetchAsStream is gone :(

Comment 17

[Tim Taubert [:ttaubert] (inactive)]

Sorry, we have no plans to bring this back. This was never intended to be used by add-ons and was implicitly slated for removal as soon as we can. Sticking to standardized methods is the way to go.

Comment 18

[noitidart]

(In reply to Tim Taubert [:ttaubert] from comment #17)
> Sorry, we have no plans to bring this back. This was never intended to be
> used by add-ons and was implicitly slated for removal as soon as we can.
> Sticking to standardized methods is the way to go.

Aw darn. Can you please mention the standard methods.

Comment 19

[Tim Taubert [:ttaubert] (inactive)]

(In reply to noitidart from comment #18)
> Can you please mention the standard methods.

I was referring to the canvas element API:

https://developer.mozilla.org/en-US/docs/Web/API/HTMLCanvasElement

Comment 20

[noitidart]

(In reply to Tim Taubert [:ttaubert] from comment #19)
> (In reply to noitidart from comment #18)
> > Can you please mention the standard methods.
> 
> I was referring to the canvas element API:
> 
> https://developer.mozilla.org/en-US/docs/Web/API/HTMLCanvasElement

Hi Tim, may you please check out this topic - https://discourse.mozilla-community.org/t/post-arraybuffer-to-loadonetab/6224

Instead of canvas.mozFetchAsStream, then adding that stream directly to the nsIMultiplexInputStream - I am now doing canvas.toBlob then readAsArrayBuffer, and then am trying to attach that buffer. However if the image is bigger then 200x200px then the post data is corrupt for some reason. :(

Comment 21

[noitidart]

Oops forgot to need info. Need info'ing Tim - hey tim, a couple of us can't figure out how to make the arraybuffer alternative work without the size issues. Can you please share with us the alternative to mozFetchAsStream, we are using it with loadOneTab so it has to be a stream.
Thanks

Comment 22

[Tim Taubert [:ttaubert] (inactive)]

Sorry, I don't know enough about our streams interfaces to be helpful here. I forgot most of what was needed to introduce mozFetchAsStream() in those years in between.

Comment 23

[noitidart]

(In reply to Tim Taubert [:ttaubert] from comment #22)
> Sorry, I don't know enough about our streams interfaces to be helpful here.
> I forgot most of what was needed to introduce mozFetchAsStream() in those
> years in between.

Aw darn no problem. Thanks for the reply. Isn't it not appropriate to deprecate things that have no alternatives? Especially in the docs when I deprecate stuff I link out the alternative. So should we leave this bug open till someone can provide an alternative solution to get data from canvas to stream in the docs?