Closed
Bug 291218
Opened 19 years ago
Closed 18 years ago
Canvas .toDataURL() method (was: src attribute)
Categories
(Core :: Graphics: Canvas2D, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: roc, Assigned: roc)
References
()
Details
Attachments
(1 file)
22.85 KB,
patch
|
Details | Diff | Splinter Review |
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).
Comment 1•19 years ago
|
||
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.
Comment 2•19 years ago
|
||
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();
Comment 4•19 years ago
|
||
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.
Assignee | ||
Comment 6•19 years ago
|
||
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.
Assignee | ||
Comment 8•19 years ago
|
||
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...)
Comment 9•19 years ago
|
||
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.
Comment 13•19 years ago
|
||
> 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)
Comment 15•19 years ago
|
||
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
Comment 17•19 years ago
|
||
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.
Assignee | ||
Comment 19•19 years ago
|
||
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
Assignee | ||
Comment 21•19 years ago
|
||
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.
Updated•19 years ago
|
Component: DOM: Level 0 → Layout: Canvas
Updated•19 years ago
|
Summary: Canvas 'src' attribute → Canvas .toDataURL() method (was: src attribute)
Depends on: 293244
Assignee | ||
Comment 22•19 years ago
|
||
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.
Comment 23•19 years ago
|
||
I'll convert this to using imgIEncoder and merge with my patch to do this work...
Assignee | ||
Comment 24•19 years ago
|
||
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.
Comment 25•19 years ago
|
||
(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...
Comment 26•19 years ago
|
||
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).
Assignee | ||
Comment 27•19 years ago
|
||
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.
Comment 28•19 years ago
|
||
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?
Assignee | ||
Comment 29•19 years ago
|
||
You'd pass the canvas element to C++ code and from C++ you can QueryInterface it to an gfxIImage or something like that.
Comment 30•19 years ago
|
||
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....
Comment 31•19 years ago
|
||
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...
Assignee | ||
Comment 32•19 years ago
|
||
(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.
Comment 33•19 years ago
|
||
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...)
Assignee | ||
Comment 34•18 years ago
|
||
This has actually already been implemented in bug 245684.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Comment 35•18 years ago
|
||
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).
Comment 36•18 years ago
|
||
http://lxr.mozilla.org/seamonkey/source/dom/public/idl/html/nsIDOMHTMLCanvasElement.idl#58
Blocks: 333613
You need to log in
before you can comment on or make changes to this bug.
Description
•