Closed Bug 291218 Opened 19 years ago Closed 18 years ago

Canvas .toDataURL() method (was: src attribute)

Categories

(Core :: Graphics: Canvas2D, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: roc, Assigned: roc)

References

()

Details

Attachments

(1 file)

Hixie specced a "src" attribute for the canvas element that lets you get a data:
URL representing a PNG encoding of the content. We should implement this because
I believe it will be incredibly useful. For example, extension authors can use
it to save images, and a Web page could implement a paint program supporting
upload of images to a server.

There is a security issue. We don't want Web pages to load images from some
other domain, paste them into a canvas, and upload the results to another Web
server.
A simple approach to preventing this would be to require drawImage() to make a
same-origin check and fail to draw images from a different origin (unless, of
course, it's running with chrome privileges).
Note that the fact that src="" is specced this way was more because I had to
spec it to do something, than because it's a good design. I believe we
definitely need some way to save image data -- however, there may be better
ways. If people have better ideas we should consider them.

Having said that, I happen to think that this is a pretty neat solution for the
Web world.
How about having a read only getter to the 'src' DOM property and just totally
ignore the 'src' attribute? Maybe another name is better to prevent confusion.
Yeah, if the intent is to let people pull it out as a data: URL, I would prefer
naming that made that more explicit, like a .getAsDataURL() method or .asDataURL
attribute.  ".src" sounds, to me, like it describes the source of the canvas
image -- which is in my mind the combination of calls made to draw on the canvas
itself, plus assets used as drawImage arguments.  And as far as <img> heritage,
I think .src is set much more often than it's read; the "copy <canvas> to <img>
by copying .src attributes" use case doesn't seem to me that it would be grossly
impaired by making the author type

myImg.src = myCanvas.getAsDataURL();
erik: sorry, my bad. I meant the .src DOM attribute, not the src="" content
attribute. The content attribute does indeed have to be totally ignored.

shaver: I like the .asDataURL idea, it does make more sense. Note that at this
point this basically means that all the HTMLImageElement members are no-ops when
implemented by HTMLCanvasElement except for width/height (and useMap/isMap, but
we don't support that yet, if we ever will).
Yeah, I wonder if we could just make HTMLCanvasElement inherit HTMLElement, and
then let the functions on the 2d-context take HTMLElement as argument instead.
This could allow passing <input type=image>, which may or may not be a good idea.
That opens a big can of worms about what to do with non-image, non-canvas
elements. Let's not go there. I think returning nothing for most of the image
DOM attributes is fine.

How about .toDataURL()? Whatever is more consistent with DOM naming conventions,
I guess...
I don't like the idea of inheriting an interface and then not implementing the
majority of the functions of it. Why not simply throw an exception, that's what
would happen if you pass a non-image element with the current interface anyway.
It's just that the exception would originate from elsewhere.
I don't like the idea of taking any element but only working on two types of
elements. But I could live with it, especially since it means less work for us
(since we currently don't implement HTMLImageElement on canvases...)
As shaver and I discussed on IRC, the much simpler solution is to make the
context simply accept both HTMLCanvasElement and HTMLImageElement on the two
methods where it matters. That's no big deal if vlad and pav agree (and should
be another bug anyway).

Regarding an alternate name for .src: I don't mind what the name really is,
.toDataURL() (a method) or .asDataURL (an attribute) are both fine to me.
Hixie: that sounds basically like what i proposed?

I don't care much either if we use a method or property. It looks like the DOM
WG used more properties then methods. Though i've always preffered using a
functioncall for stuff that are slow since they have a slower 'feel' to them.
If we're trying to preserve drawImage() here, we can simply state that drawImage
should take either an Image element or a Canvas element.  No need to define
inheritance between the two.

I do like toDataURL(), though (method, not attribute, please, as computing it is
bound to be expensive).
> we can simply state that drawImage should take either an Image element or a
> Canvas element.

Yes, but the question is what type should that argument be of? See comment 5 and on.

> I do like toDataURL(), though (method, not attribute, please, as computing it
> is bound to be expensive).

While I agree with you things like .textContent and .baseURI are properties in
the current DOM. hasChildNodes() is interestingly enough a method though.
> Yes, but the question is what type should that argument be of?

Both. See the updated spec.
IMHO that is a bad idea. Javascript doesn't support overloading like that so the
implementation will have to create a single function that accepts any type of
argument and then check the type manually. This is more work for the
implementation and the end result is the same as accepting a more strict
datatype like DOMObject or HTMLElement (or something inbetween)
The drawImage() method is already overloaded anyway.
Yes, and i'd prefer if we changed that. But i'm guessing that might give
compatibility problems :(

In any event I don't see a reason to expand the problem to more methods
I don't understand the difference between what's in the spec and what you're
proposing, except that the spec would be more complicated (due to the increased
number of potential error cases to cover) if we said just HTMLElement instead of
HTMLCanvasElement and HTMLImageElement.
Yay, that's a long sentence :)

The difference between what's in the spec and what i'm proposing is basically
that what's in the spec is more complicated.

With the spec the way it is now the implementation will have to write a function
that can take any type of arguments. Then manually check that each and every one
of the argument is the right datatype, as well as that there are the right
number of arguments. And then throw errors as appropriate for every one of the
potential errors.

With my suggestion the implementation can just use whatever standard mechanism
for checking datatypes for normal functioncalls there is and then check that the
passed element is either an image of canvas.

Of course, we could just keep the spec the way it is and let the implementation
do what I suggest. I.e. basically just let the spec be the way it is as a way of
documentation. 

But then I don't see why we couldn't write the spec that way too. IMHO two
overloaded functions (or 6 in the case of drawImage) are no easier to understand
 then a sentence saying that the element should be either an image or canvas.
Ideally we'd give the methods different names and avoid overloading altogether,
but I guess Apple has prevented that.

I'd leave the spec as is because some languages do support overloading and for
those languages the spec is appropriate.
Given that we're designing this interface primarily for javascript I would
rather that we avoided overloading as much as possible. See also bug 290845
It's not really related to bug 290845 (which I think should not have been marked
INVALID, but that's another story). We *should* avoid overloading in general,
but given that we can't in this case because of Safari's decisions, we may as
well write in the spec exactly what overloading combinations are allowed for the
sake of whoever can make use of that information.
Component: DOM: Level 0 → Layout: Canvas
Summary: Canvas 'src' attribute → Canvas .toDataURL() method (was: src attribute)
Attached patch fixSplinter Review
This implements toDataURL().

I notice that the save process takes a little less than 1 second for a very
complex 600x600 image with transparency (and not much less without
transparency). This is quite slow considering my machine... I haven't done any
profiling to see why it's slow. I added toDataURLFast() as a quick hack to try
setting the compression level to zero; it doesn't seem to be any faster. For
some applications this speed is probably fine; for others, it might not be. We
may indeed want some sort of asynchronous API here, if we can't get the save
performance up.

Another option is to return a PPM data: url and reenable our PPM decoder.
However, for many uses of toDataURL (e.g., uploading an image to a server), you
do want to compress the image so the compression cost must be paid somewhere
and the PPM approach doesn't help.

I'm leaving town for a couple of weeks. If someone else wants to push this
along while I'm away, that'd be great.
I'll convert this to using imgIEncoder and merge with my patch to do this work...
This patch also has some changes to nsAutoBuffer that we'd better not lose ...
e.g. it currently corrupts memory if you call EnsureElemCapacity more than once
with a large size.
(In reply to comment #23)
> I'll convert this to using imgIEncoder and merge with my patch to do this work...

Any news on this? I'd be happy to test patches...
Pavlov, roc, is there a chance that this makes it into Gecko 1.8/Firefox 1.5?
What needs to be done with the attached patch? Does it require special graphics
know-how?

I realize that bug 293244 is likely not fixable in this time frame. How about
restricting .toDataURL() to chrome callers for now? For App UI and extensions,
such a protection is certainly not neccessary.

I'd like to emphasize that a way to access the image data makes the Canvas
element much more valuable to extension and XULRunner developers. Actually, some
really interesting ideas basically depend on this, e.g. visual page history
(saving the thumbnails to disk).
We had a talk about this recently. It's possible for a C++ component to get
access to the data already. There probably isn't enough time to fix this bug in
a way that would be usable by Javascript.
I too pretty much like to see a way to save the canvas content to a file in 1.5
or in XULRunner. I don't really understand why this isn't at least implemented
for chrome applications. In it's current state it's a killer feature but nearly
useless in most of cases.

> We had a talk about this recently. It's possible for a C++ component to get
> access to the data already.

How? Any example or allready implemented stuff?
You'd pass the canvas element to C++ code and from C++ you can QueryInterface it
to an gfxIImage or something like that.
Regarding comment # 28 and comment # 29, I recently worked with Kathy Brade to implement a C++ component to save a canvas' contents as a PNG file (we use it in our Pearl Crescent Page Saver extension -- http://pearlcrescent.com/products/pagesaver/).  The "path" we ended up with was:

  imgIContainer = nsICanvasElement->GetCanvasImageContainer();
    gfxIImageFrame = imgIContainer->GetCurrentFrame();
      imgFrame->GetImageData().

Of course there may be other ways to get from Point A to Point B....
I sincerely hope this bug gets revisited for the next release of Firefox. Not only would it be extremely useful for web applications, it would allow canvas to have some visibility in CSS. You would be able to set a background image, etc. via a data url generated from canvas.

I guess I dont understand what the security issues are, something about submitting an image from one domain as an image from another? Isnt that already possible by just downloading an image and uploading it to another server?

Anyway, I'm hoping that this bug gets another look and is considered for inclusion in a point release for Firefox...
(In reply to comment #31)
> I guess I dont understand what the security issues are, something about
> submitting an image from one domain as an image from another? Isnt that already
> possible by just downloading an image and uploading it to another server?

Currently a regular Web page can display an image from another domain (e.g., the user's intranet) if the URL is known, but it cannot access information about the image (other than whether it exists and if so, what its size is) to send back to its origin. If we create a mechanism whereby a Web page can fetch an image from the intranet and transmit the contents anywhere, then we will be opening a much bigger can of worms. E.g., imagine if someone's intranet has a dynamically generated image showing the latest financial results, at a predictable URL ... we wouldn't want bad guys to be able to publish a Web page that fetches that image and forwards it wherever.
I see. So if the same-origin check is in place, would that clear the way for this to get turned on in a future release?  I ask because I hear folks talking about trying to create js based png renderers or js based toDataURL workarounds, and it seems like it would be better to have this living in the native codebase as opposed to js workarounds.

Anyway - I just want to say that this is one feature that is on the top of my wish list ... (This and getPixel...)

This has actually already been implemented in bug 245684.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
I guess you're talking about this:
http://lxr.mozilla.org/seamonkey/source/content/canvas/public/nsICanvasRenderingContextInternal.h#51

But there's still no scriptable way to do this (which is why, AIUI, this bug was filed in first place).
Depends on: 245684
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: