Closed Bug 288714 Opened 20 years ago Closed 20 years ago

HTML canvas impl

Categories

(Core :: Graphics: Canvas2D, defect)

Product:
Core
Component:
Graphics: Canvas2D
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: vlad, Assigned: vlad)

References

(
URL
)

Details

Attachments

(2 files, 5 obsolete files)

html-canvas-4.patch
120.32 KB, patch
brendan
: approval1.8b2+
Details | Diff | Splinter Review
canvas-fix-classinfo.patch
5.34 KB, patch
sicking
: review+
Details | Diff | Splinter Review
This patch moves the <canvas> implementation from XUL Box Object ghetto land into a real HTML element, complete with hooks into DOM goodness and better paint semantics, where we might actually not be slow as molasses on most platforms. It's turned on whenever --enable-cairo is turned on, and builds with the in-tree cairo (and, in theory, with any system cairo). Stuart's working on making cairo build on windows, because it's a tossup as to whether that will work right now. The patch defines a few new things, along with associated sprinkled code to make things like the html parser recognize <canvas> as a tag, to get the right DOM ClassInfo for the various canvas interfaces, etc. layout/generic/nsHTMLCanvasFrame -- a simple frame impl that can display a single ImageFrame dom/public/idl/html/nsIDOMHTMLCanvasElement.idl -- the base <canvas> element content/html/content/src/nsHTMLCanvasElement.cpp -- <canvas> HTML element impl content/canvas/* -- new content subdir composing the Canvas interfaces, and the future place for future canvas context work (e.g. 3d) Could use some reviews especially on the nsHTMLCanvasFrame and HTMLCanvasElement impl.
Attached patch html-canvas.patch (obsolete) — — Splinter Review
Attachment #179340 - Flags: superreview?(dbaron)
Attachment #179340 - Flags: review?(pavlov)
The code to convert from an cairo_image_surface to a gfxIImageFrame isn't going to work on windows or mac. Windows needs the data to be upside down BGR and mac needs XRGB (or is it RGBX?). The other option here would be to get the nsIRenderingContext's nsIDrawingSurface and QI it to platform specific versions like nsIDrawingSurfaceWin, get the DC and create a windows cairo surface and move_to(), show_surface() the image surface on to it. That said, it doesn't look like all platforms have their own drawing surface interface...
Attached patch html-canvas-1.patch (obsolete) — — Splinter Review
Patch fixes the content element - frame relationship to be more sane; things like "display: none" and the like are correctly handled now, as well as resizes of the canvas. CSS colors are also correctly parsed now (requires separate ParseColorString patch). The image frame rendering bits pavlov pointed out aren't fixed yet; that's coming up next.
Attachment #179340 - Attachment is obsolete: true
Attachment #179340 - Flags: superreview?(dbaron)
Attachment #179340 - Flags: review?(pavlov)
+SDK_XPIDLSRCS += nsIDOMHTMLCanvasElement.idl that file seems to be missing from the patch?
And even if it were in the patch, it probably doesn't belong in SDK_XPIDLSRCS, since that is only for frozen interfaces.
Attached patch html-canvas-2.patch (obsolete) — — Splinter Review
(Forgetting the -N option sucks) More fixes, including drawImage support and handling of MACOSX/WIN RGB_A8-only-not image formats. Only thing missing now is pattern/gradient objects. Most of the code should be ready for review (especially frame/content bits)
Attachment #179441 - Attachment is obsolete: true
+ aStr.Assign(NS_ConvertUTF8toUTF16(nsPrintfCString(100, "#%02x%02x%02x", CopyUTF8toUTF16
In the cairo_* function calls, about half use |foo()| while the other half use |foo ()|. Aside from that, the patch looks correct. I think we should get it in the tree for some further testing.
Comment on attachment 179888 [details] [diff] [review] html-canvas-2.patch >Index: layout/generic/nsHTMLCanvasFrame.cpp >+ * Portions created by the Initial Developer are Copyright (C) 1998 2004, perhaps? >+ * Contributor(s): Might want to do an "(original author)" line with email address. >+ fprintf (stderr, "*** NS_NewHTMLCanvasFrame\n"); >+ >+ NS_PRECONDITION(aNewFrame, "null OUT ptr"); >+ if (nsnull == aNewFrame) { >+ return NS_ERROR_NULL_POINTER; >+ } No need for this (especially the fprintf). >+NS_INTERFACE_MAP_BEGIN(nsHTMLCanvasFrame) >+ NS_INTERFACE_MAP_ENTRY(nsIFrame) >+NS_INTERFACE_MAP_END >+ >+NS_IMETHODIMP_(nsrefcnt) nsHTMLCanvasFrame::AddRef(void) >+{ >+ NS_WARNING("not supported for frames"); >+ return 1; >+} >+ >+NS_IMETHODIMP_(nsrefcnt) nsHTMLCanvasFrame::Release(void) >+{ >+ NS_WARNING("not supported for frames"); >+ return 1; >+} No need for any of this either (and the QueryInterface you inherit is better). >+NS_IMETHODIMP >+nsHTMLCanvasFrame::Destroy(nsPresContext* aPresContext) >+{ >+ return nsSplittableFrame::Destroy(aPresContext); >+} No need for this either (doesn't change anything). >+ >+NS_IMETHODIMP >+nsHTMLCanvasFrame::Init(nsPresContext* aPresContext, >+ nsIContent* aContent, >+ nsIFrame* aParent, >+ nsStyleContext* aContext, >+ nsIFrame* aPrevInFlow) >+{ >+ nsICanvasElement *canvas; >+ CallQueryInterface(aContent, &canvas); >+ if (!canvas) >+ return NS_ERROR_FAILURE; This should probably be NS_ENSURE_TRUE so it gives an assertion. Also, it's perhaps better to check NS_SUCCEEDED() on the result of QueryInterface instead of a null-check. >+NS_METHOD >+nsHTMLCanvasFrame::HandleEvent(nsPresContext* aPresContext, >+ nsGUIEvent* aEvent, >+ nsEventStatus* aEventStatus) >+{ >+ return nsSplittableFrame::HandleEvent(aPresContext, aEvent, aEventStatus); >+} >+ >+ >+NS_METHOD >+nsHTMLCanvasFrame::GetCursor(const nsPoint& aPoint, >+ nsIFrame::Cursor& aCursor) >+{ >+ return nsFrame::GetCursor(aPoint, aCursor); >+} These don't seem to serve any purpose either. >Index: layout/generic/nsHTMLCanvasFrame.h Same comments on extra methods as for the .cpp file. >+ virtual void GetDesiredSize(nsPresContext* aPresContext, >+ const nsHTMLReflowState& aReflowState, >+ nsHTMLReflowMetrics& aDesiredSize); No need for this to be virtual since you're not inheriting from nsLeafFrame. In fact, probably no need for it at all. More later, perhaps...
And there are a bunch of places where you used NS_METHOD instead of NS_IMETHODIMP.
Blocks: 279908
Attached patch html-canvas-2.patch (obsolete) — — Splinter Review
Updated with dbaron's feedback, and also updated to latest tree changes.
Attachment #179888 - Attachment is obsolete: true
Attached patch html-canvas-3.patch (obsolete) — — Splinter Review
Forgot -N, again. Grr.
Attachment #180613 - Attachment is obsolete: true
Will be landing this tonight, hopefully; any comments, please speak up :).. pav cought a NS_IMETHODIMP, and there's also some extraneous #if 0's and fprintf's in that patch that i'll take out before committing. (Various people have looked over bits and pieces of this as appropriate.)
What's the purpose of the nsICanvasRenderingContext interface? Couldn't you make nsIDOMHTMLCanvasElement.getCanvas return an nsISupports and then just have nsICanvasRenderingContext2D. The interface is less then ideal with one scripted and one unscripted function. Also nsICanvasRenderingContext2D should probably be named nsIDOMCanvasRenderingContext2D to follow the naming convention of other DOM interfaces. Also we do some magic on interfaces whos names start with 'nsIDOM' which you probably want ('CanvasRenderingContext2D.prototype.foo = bar' would work in script for example). The same applies to nsICanvasPattern and nsICanvasGradient
sicking: I hate to be a name pest, but don't we also put nsIDOM-prefixed *non-w3c-standard* interfaces in an nsIDOMNS-prefixed sub-space? /be
+nsHTMLCanvasElement::GetContext(const nsAString& aContextId, + nsICanvasRenderingContext **aContext) +{ + if (mCurrentContextId.IsEmpty()) { + nsCString ctxString("@mozilla.org/content/canvas-rendering-context;1?id="); + ctxString.Append(NS_LossyConvertUTF16toASCII(aContextId)); + + mCurrentContext = do_CreateInstance(nsPromiseFlatCString(ctxString).get()); Is this safe to do given that aContextId comes from script? I can't think of any risks offhand, but it feels a bit scary to instantiate a contract-id partially comming from script.
It seems we only do that when we want to extend objects that already have a w3c interface, like nsIDOMNSRange. We havn't done it for nsIDOMWindow or nsIDOMPluginArray. I'm not entierly sure when we want to use nsIDOM and when we don't, i'd love to get a second oppinion from jst or peterv.
+ ctxString.Append(NS_LossyConvertUTF16toASCII(aContextId)); LossyAppendUTF16toASCII
(In reply to comment #16) > + nsCString ctxString("@mozilla.org/content/canvas-rendering-context;1?id="); > + ctxString.Append(NS_LossyConvertUTF16toASCII(aContextId)); > > Is this safe to do given that aContextId comes from script? I can't think of any > risks offhand, but it feels a bit scary to instantiate a contract-id partially > comming from script. I /think/ it's ok, because we have a full contractid before the bit we just append as the "id" element. To be sure, we can probably limit aContextId to [A-Za-z0-9_-].
The entire thing feels a bit overengineered though, are there really enough frozen interfaces for anyone to actually write a plugin for this anyway? If not a simple if(aContextId.EqualsLiteral('id') { foo = new nsContext2D() } would do just as well IMHO. It doesn't matter that much to me though, as long as we're not exposing ourselvs to security problems.
I think we're safe here also because we force it to QI to the nsICanvasRenderingContext interface. Anything with that contract-ID prefix which QIs appropriately had better be safe to hand back, because that's the only reason to have that contract ID. If we had meaningful contract-ID parameterization, that might be worth worrying about a bit more, but I think we're fine here, and finer still with a [A-Za-z0-9_-] clamp.
nsICanvasRenderingContext is scriptable too though, but if we move Init to nsICanvasRenderingContextInternal (which i think we should do anyway) we should be fine.
As our long history with Java plugins has shown, people can write useful extensions that are locked to a specific version or set of versions, without needing the benefit of universally frozen interfaces. I think XForms is in the same situation.
(In reply to comment #22) > nsICanvasRenderingContext is scriptable too though, but if we move Init to > nsICanvasRenderingContextInternal (which i think we should do anyway) we should > be fine. Yep, this is done; I got rid of nsICanvasRenderingContext entirely, there's just Internal and 2D now.
Patch that will be landing shortly. Changes: - <canvas> is now always valid; the only thing that's enabled by --enable-cairo is the 2D context itself. This lets us do non-cairo contexts, and lets people add in contexts via extensions. - the context ID is clamped to [A-Za-z0-9_-] - getContext() now returns nsISupports, and nsIDOMCanvasRenderingContext2D.idl lives in dom/public/idl/canvas. - no more nsI(DOM)CanvasRenderingContext; nsIDOMCanvasRenderingContext2D inherits from nsISupports directly, and just implements the "canvas" attribute that was on nsIDOMCanvasRenderingContext before.
Attachment #180617 - Attachment is obsolete: true
Comment on attachment 181013 [details] [diff] [review] html-canvas-4.patch Yay, a=me for 1.8b2. /be
Attachment #181013 - Flags: approval1.8b2+
dbaron: (with reference to a question you asked on IRC) According to the WHATWG spec, <canvas> has an end tag and the contents are used as fallback content. This is critical for accessibility and important for backwards compatibility. Apple didn't implement this; their <canvas> element has no end tag and renders both the fallback content and the canvas. (They support alt="", but that's useless as a backwards compatibility thing since it means browsers have to support <canvas> in order to not support it, as it were, which is silly.) I strongly suggest that we stick with what is in the spec, so that accessibility is addressed and we have a backwards compatibility story. While there is little to no content out there, we can still do the right thing and force Apple to change their implementation.
Comment on attachment 181013 [details] [diff] [review] html-canvas-4.patch >Index: dom/src/base/nsDOMClassInfo.cpp >=================================================================== >@@ -577,16 +582,18 @@ static nsDOMClassInfoData sClassInfoData > NS_DEFINE_CLASSINFO_DATA(HTMLBaseElement, nsHTMLElementSH, > ELEMENT_SCRIPTABLE_FLAGS) > NS_DEFINE_CLASSINFO_DATA(HTMLBaseFontElement, nsHTMLElementSH, > ELEMENT_SCRIPTABLE_FLAGS) > NS_DEFINE_CLASSINFO_DATA(HTMLBodyElement, nsHTMLElementSH, > ELEMENT_SCRIPTABLE_FLAGS) > NS_DEFINE_CLASSINFO_DATA(HTMLButtonElement, nsHTMLElementSH, > ELEMENT_SCRIPTABLE_FLAGS) >+ NS_DEFINE_CLASSINFO_DATA(HTMLCanvasElement, nsHTMLElementSH, >+ DOM_DEFAULT_SCRIPTABLE_FLAGS) ELEMENT_SCRIPTABLE_FLAGS
Damnit, missed that when I moved the Canvas element to live with the rest of the elements. Will fix, thanks! This landed tonight, much to the chagrin of some tinderboxes; I'll be fixing the fallout/issues as they occur.
Comment on attachment 181013 [details] [diff] [review] html-canvas-4.patch Ian: What should happen when a canvas element is cloned? Should that also clone the current image? Vlad: A few things we talked about on irc that's still not changed: nsHTMLCanvasElement::GetAttributeChangeHint is still there. nsHTMLCanvasElement::GetWidthHeight no longer needs to deal with attributes of type eString since you now implement ParseAttribute Removing the width or height attribute won't clear the image. You need to implement nsHTMLCanvasElement::UnsetAttr and make nsHTMLCanvasElement::UpdateImageContainer nuke the current image rather then just bailing if width/height is 0. nsHTMLCanvasElement::IsAttributeMapped doesn't return true for width and height. Things i found in the new changes: You might as well make mCurrentContext an nsCOMPtr<nsICanvasRenderingContextInternal>. Just make sure you QI it (rather then cast it) to nsISupports before you return it from getContext so that you return the COM-correct pointer. I'm not sure it's a great idea to support attribute-mapping for all attributes that <img> supports. Many of them are just there for backwards compatibility and there's obviously no such need for canvas. Ian, do you have an oppinion here? What does apple do. I also noticed that you need to move the new eDOMClassInfo_HTMLCanvasElement_id down to the end of the enum. See the comment on line 199.
sicking: Cloning a <canvas> should just do a DOM clone, which always drops the non-XML data, as far as I can tell. As far as the DOM interface, according to the WHATWG spec HTMLCanvasElement inherits from HTMLImageElement, but HTMLImageElement doesn't have any of the presentational crap from yore. Some attributes in particular have special meaning for <canvas>, e.g. .src doesn't attribute-map to src="", it just returns a data: URI of a PNG that represents the current rendering (and does nothing on setting).
I'm not sure that there is any spec that says what should happen with non-DOM data during a .clone actually. And it's even debatable whether the image is DOM data in a canvas since you actually create it through DOM. I would think it could be usefull to have the ability to copy <canvas> images using .clone. Sorry, should have been more clear what i ment by mapped attributes. I was talking about attributes that affects style, such as border="5", vspace="10" or align="left". Should such attributes work on <canvas>. And if so, which ones? All the ones that work on <img>?
> it could be usefull to have the ability to copy <canvas> images using .clone. You can drawImage() one on the other. As for mapping presentational attributes to style, who knows. I'd ideally say support none of them.
I'm still not convinced that .clone should create an empty image, but this bug probably isn't the best place to discuss this. Wrt style-mapped attributes. I'd missed that HTMLCanvasElement inherits HTMLImageElement. Since that's the case I think we should at least map 'border', 'hspace' and 'vspace' so that those DOM-properties behave like on <img>. As well as 'width' and 'height' of course. Other then that I agree that we should probably support as few as possible, meaning none.
(In reply to comment #30) > Vlad: A few things we talked about on irc that's still not changed: Hmm, some of these may have been due to my misunderstanding: > nsHTMLCanvasElement::GetAttributeChangeHint is still there. Hmm, how else do I get the frame to reconstruct itself? I thought we decided that until things get rewritten to allow the reuse of the existing frame, that leaving this in should be fine for now. > nsHTMLCanvasElement::GetWidthHeight no longer needs to deal with attributes of > type eString since you now implement ParseAttribute I left that in to avoid rocking the boat; but if it's ensured now that I can't get non-integers (as, I guess, it is with ParseAttribute) I'll take it out. > Removing the width or height attribute won't clear the image. You need to > implement nsHTMLCanvasElement::UnsetAttr and make > nsHTMLCanvasElement::UpdateImageContainer nuke the current image rather then > just bailing if width/height is 0. Whoops, yes; forgot about this part. Will fix shortly. > nsHTMLCanvasElement::IsAttributeMapped doesn't return true for width and > height. Right; they're not mapped, so that we can support things like what roc suggested in the future (specifying a different width/height for presentation using CSS). > Things i found in the new changes: > > You might as well make mCurrentContext an > nsCOMPtr<nsICanvasRenderingContextInternal>. Just make sure you QI it (rather > then cast it) to nsISupports before you return it from getContext so that you > return the COM-correct pointer. Will do. > I also noticed that you need to move the new eDOMClassInfo_HTMLCanvasElement_id > down to the end of the enum. See the comment on line 199. Argh! I completely didn't see that comment (in the middle of the enum, no less ;). And I spent all the time moving it up into its alphabetical location in the HTML Element block, too... I'll fix asap today. Thanks!
(In reply to comment #31) > .src doesn't > attribute-map to src="", it just returns a data: URI of a PNG that represents > the current rendering (and does nothing on setting). hm, in combination with drawImage, isn't that a security problem? You can read images from intranet servers that way and send it where you want...
(In reply to comment #35) > > nsHTMLCanvasElement::GetAttributeChangeHint is still there. > > Hmm, how else do I get the frame to reconstruct itself? I thought we decided > that until things get rewritten to allow the reuse of the existing frame, that > leaving this in should be fine for now. By making IsAttributeMapped return true for width/height we'll reflow the frame as needed when the attribute changes, which I think will be enough for things to work. > > nsHTMLCanvasElement::GetWidthHeight no longer needs to deal with attributes > > of type eString since you now implement ParseAttribute > > I left that in to avoid rocking the boat; but if it's ensured now that I can't > get non-integers (as, I guess, it is with ParseAttribute) I'll take it out. It's ensured that it'll be an integer if it can be parsed as such. But you still need to check the datatype just in case someone sets width="hello" (in which case we should treat it as if the attribute isn't set). > > Removing the width or height attribute won't clear the image. You need to > > implement nsHTMLCanvasElement::UnsetAttr and make > > nsHTMLCanvasElement::UpdateImageContainer nuke the current image rather then > > just bailing if width/height is 0. > > Whoops, yes; forgot about this part. Will fix shortly. Actually, I just noticed that the spec says that there are default values for width and height, so we should just use that if the attribute is missing. You still need to implement UnsetAttr though, so that we revert to the default then. You probably want to create the image-frame lazily though, so that we don't create it a bunch of times during parsing of the element. > > nsHTMLCanvasElement::IsAttributeMapped doesn't return true for width and > > height. > > Right; they're not mapped, so that we can support things like what roc > suggested in the future (specifying a different width/height for presentation > using CSS). Even with rocs suggestion we want the attributes to be mapped so that they act as a default value if no css is specified. So in other words: <canvas width=100 height=100> <canvas width=100 height=100 style="width:50px; height:50px"> The first will give a 100x100 display, but in the second the stylerule will override the mapped attributes and give a 50x50 display. Makes sense?
So I'm trying to follow what actually landed here (which I gather is different from the last patch in this bug?). A few issues I see offhand: 1) nsHTMLCanvasElement::GetWidthHeight seems to be doing unnecessary (and incorrect) work. ParseAttr will ensure here that the width/height are stored as integers if they're valid nonnegative integers. So this method shouldn't have to mess with strings. The fact that it does means it'll return negative widths/heights if those are set; is that desirable Furthermore, since this isn't dealing with nscoords it should be using nsIntSize, not nsSize. 2) If #1 is fixed, it looks like you can just use the standard "int attr" macros (NS_IMPL_INT_ATTR_DEFAULT_VALUE, to be exact) to implement {Get/Set}{Width/Height}. 3) Could we please document the idl? nsIDOMCanvasRenderingContext2D.idl is a perfect example of the sort of interface we _don't_ want to be checking in, imo. We have far too many such written years ago and are now paying the prince in developer time and effort. If desired I can write up a summary of issues with the documentation of this interface (all of which come down to "there isn't any", but I can point out all the places that I think should have some). If this is supposed to be the same as the similarly named interface in the whatwg spec, the IDL should say so (and then the whatwg spec needs vast improvement; I can't see how what's specified there can possibly be implemented in a reasonable way without constantly asking Hixie what he means). 4) Width and height don't seem to be mapped into style in the code that was checked in. That's not good, because it sorta violates the way presentational attributes should work in CSS. 5) Do we really need a framechange on width/height changes? I would have thought a reflow would be sufficient. 6) In GetContext(), do we really want to throw INVALID_ARG on OOM? 7) The canvas frame seems to completely ignore CSS for sizing. Is this desired? 8) The canvas frame should implement GetFrameName (from nsIFrameDebug). 9) The canvas frame claims to be splittable, and reflows as if it's splittable, then completely flubs painting if it's actually split. Either fix the painting, or don't make it splittable, please.
Comment on attachment 181053 [details] [diff] [review] canvas-fix-classinfo.patch IMO it'd be fine to just move the enum and let the code be in a more logical order. r=me either way though.
Attachment #181053 - Flags: review+
Boris, most of the things you bring up is stuff that is already worked on (though 8 and 9 were new to me). I do defenetly agree about the context2D interface sucking rocks though. The optional arguments to drawImage is simply not acceptible, and using nsIVariants (or actually nsISupports which is what we map DOMObject to) is going to be a pain in the a** for non-script users. But lets fight that out in bug 290845 that I just filed.
> hm, in combination with drawImage, isn't that a security problem? You can read > images from intranet servers that way and send it where you want... Valid point, drawImage() should probably do a same-origin check.
Regarding what the <img> attributes should do on <canvas>: I really think they should do nothing. The current text in the spec is: | The alt DOM attribute shall return the same value as the textContent DOM | attribute. On setting it must do nothing. | | The src attribute must return a data: URI containing a representation of the | image as a PNG file. [PNG] On setting it must do nothing. | | The useMap and isMap DOM attributes inherited from the HTMLImageElement | interface shall do the same as for img elements. | | All the other DOM attributes — align, border, hspace, vspace, longDesc, and | name — must do nothing on setting and return the empty string on getting. It's not a problem is we don't implement useMap, isMap, and src for the first (or even later) versions straight away -- but I really don't think we should encourage the use of the presentational attributes. Those should IMHO just do nothing. It would be bad to introduce more presentational crap in 2005.
Well, I do agree introducing presentational attributes in 2005 is bad. But i bet that we'll end up getting bugs filed against us if there are DOM properties on canvas that don't do anything. The alternative is to not let HTMLCanvasElement inherit HTMLImageElement, but that'd uglify the nsIDOMCanvasRenderingContext2D interface even more.
How about making HTMLImageElement and HTMLCanvasElement both inherit from a common ancestor? Would that work?
Specwise it would, but we couldn't change that in mozilla since HTMLImageElement is a frozen interface.
Changing the inheritance hierarchy in this way would affect ABI compatibility? (even if the resulting interface was the same?) Pity. Ok, what else can we do? Do we really think people are going to try to use these DOM attributes and complain when they do nothing?
I'm not sure whether the binary C++ ABI would break (it might depend on platform). However i'm not sure if that is the only compatibility we're concerned about. In general we don't want to change frozen interfaces at all and we're talking about some pretty big surgery here. Even if the net sum of the changes might land on zero. But i'm also less happy about chaning W3C defined interfaces like this. In the end I guess I just think that adding these presentational attributes won't cause that much harm. HTML (and the current incarnation of XHTML) won't be able to forbid using presentational attributes anytime soon anyway. And all implementors should have code to do attribute mapping so it wouldn't be too big of a burdon. And yes, I do think that people would use these properites and complain about them not working.
mozillaZine ( http://www.mozillazine.org/talkback.html?article=6461 ) is reporting this as having been already checked in, which it doesn’t appear to be here. Anyway, is the <canvas> spec stable enough to land this on the trunk, given that WhatWG spec is still in Working Draft and says "It is very wrong to cite this as anything other than a work in progress. Do not implement this in a production product. It is not ready yet! At all!"? If there is even the slightest possibility of the spec on <canvas> changing, I don’t think this should land on any of the production builds.
(In reply to comment #49) > mozillaZine ( http://www.mozillazine.org/talkback.html?article=6461 ) is > reporting this as having been already checked in, which it doesn’t appear to be > here. > > Anyway, is the <canvas> spec stable enough to land this on the trunk, Yes. We are turning canvas on -- it's already on by now. Get over it. Nothing in a relatively new spec is set in concrete, but that does not prevent useful work from being built on it. This is how the Internet protocols, and then the web, were built. Welcome back to incremental innovation. The lull of six years of monopoly-induced stagnation is over. /be
Changing the interface hierarchy of frozen interfaces is verboten.
Is it intended that the canvas element cannot be styled (yet)? <canvas width="200" height="200" style="border: 1px solid black;"> doesn't get a border (using Firefox 2005-04-20 nightly)...
Depends on: 291216
Note: I'm going to close this bug shortly as the bulk of the canvas impl landed; the issues remaining that are in this patch I'm going to split off into separate bugs, so I can tackle them one at a time. Please file any new issues into separate bugs, Layout: Misc component, assign to me. Thanks!
Please make a note here of all the bugs that you file
Bugs that have been filed so far regarding CANVAS: * bug 291216 - canvas should not be an empty element * bug 291218 - Canvas 'src' attribute * bug 291262 - styling the CANVAS element doesn't work
No longer depends on: 291216
* bug 291285 - <canvas> with no width/height crashes
Component: Layout: Misc Code → Layout: Canvas
QA Contact: layout.misc-code → layout.canvas
* bug 293219 - Canvas mixes up RGB order in fillStyle and strokeStyle properties * bug 293221 - CanvasRenderingContext2D has wrong default property values
* bug 293225 - Default height of CANVAS should be 150 * bug 293226 - Antialiasing uses wrong colors with strokeRect() on Canvas element
* bug 293239 - Canvas misdraws content using style.height
* bug 293244 - canvas drawImage needs sameorigin checks
* bug 293249 - Don't pretend to support canvas when cairo isn't built
bug 293248 - <canvas> not working in beast Installer builds
* bug 293259 - Canvas should not have an opaque white background by default
* bug 293267 - When called via onclick, canvas rendering is far slower than called via onload
* Bug 293306 - canvas leaks lots of memory
* Bug 293353 - alpha transparency renders wrong colors [Win]
* Bug 293382 - canvas paints unrelated colors at borders of rotated content
Resolving this bug; Canvas has its own "Core -> Layout: Canvas" component now, so no need to keep track of canvas bugs here.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Blocks: 290392
*** Bug 102285 has been marked as a duplicate of this bug. ***
The Reflow() impl checked in in this bug has a GetPrevInFlow() check which will always test false. You should just remove that code....
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: