Closed Bug 1206030 Opened 4 years ago Closed 4 years ago

Remove nsIDOMHTMLCanvasElement::MozFetchAsStream()

Categories

(Core :: Canvas: 2D, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: ttaubert, Assigned: ttaubert)

Details

(Keywords: addon-compat, dev-doc-complete)

Attachments

(1 file)

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.
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Attachment #8662884 - Flags: review?(Ms2ger)
Summary: Remove nsIDOMHTMLCanvasElement::mozFetchAsStream() → Remove nsIDOMHTMLCanvasElement::MozFetchAsStream()
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.
Attachment #8662884 - Flags: review?(jst)
Attachment #8662884 - Flags: review?(Ms2ger)
Attachment #8662884 - Flags: feedback+
(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 on attachment 8662884 [details] [diff] [review]
0001-Bug-1206030-Remove-nsIDOMHTMLCanvasElement-mozFetchA.patch

I like removal of code we no longer need! r=jst
Attachment #8662884 - Flags: review?(jst) → review+
https://hg.mozilla.org/mozilla-central/rev/045b1309490f
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Thanks Florian!
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?
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.
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.
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.
(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.
Looks like I might have missed a few. Not sure how often we update the add-on MXR.
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?
(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 :(
Flags: needinfo?(ttaubert)
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.
Flags: needinfo?(ttaubert)
(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.
Flags: needinfo?(ttaubert)
(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
Flags: needinfo?(ttaubert)
(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. :(
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
Flags: needinfo?(ttaubert)
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.
Flags: needinfo?(ttaubert)
(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?
You need to log in before you can comment on or make changes to this bug.