I also think we need to change the packages. The new Elements should go with the other Elements: com.google.gwt.dom.client - CanvasElement, MediaElement
Canvas stuff: com.google.gwt.canvas.client // widget stuff - Canvas com.google.gwt.canvas.dom.client // non-element JSOs - CanvasGradient, CanvasPattern, CanvasPixelArray, Context, Context2d, CssColor, ImageData, IsFillStyle, IsStrokeStyle, TextMetrics The media stuff can get its own package: com.google.gwt.media.client - MediaWidget (when we create one) com.google.gwt.media.dom.client - MediaError, TimeRanges IsSupported should go in user: com.google.gwt.user.client - IsSupported http://gwt-code-reviews.appspot.com/1082801/diff/24001/25007 File user/src/com/google/gwt/html5/canvas/client/CanvasPixelArray.java (right): http://gwt-code-reviews.appspot.com/1082801/diff/24001/25007#newcode32 user/src/com/google/gwt/html5/canvas/client/CanvasPixelArray.java:32: public class CanvasPixelArray extends JavaScriptObject { Per the spec, CanvasPixelArray should have getLength(). http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#dfnReturnLink-0 "The CanvasPixelArray object thus represents h×w×4 integers. The length attribute of a CanvasPixelArray object must return this number." http://gwt-code-reviews.appspot.com/1082801/diff/24001/25009 File user/src/com/google/gwt/html5/canvas/client/Context2d.java (right): http://gwt-code-reviews.appspot.com/1082801/diff/24001/25009#newcode226 user/src/com/google/gwt/html5/canvas/client/Context2d.java:226: boolean anticlockwise) /*-{ It looks like the spec says its optional: http://www.whatwg.org/specs/web-apps/current-work/complete/the-canvas-element.html#canvas-context-2d Look for the arc method in the gray box. http://gwt-code-reviews.appspot.com/1082801/diff/24001/25010 File user/src/com/google/gwt/html5/canvas/client/CssColor.java (right): http://gwt-code-reviews.appspot.com/1082801/diff/24001/25010#newcode64 user/src/com/google/gwt/html5/canvas/client/CssColor.java:64: public String asString() { JavaDoc http://gwt-code-reviews.appspot.com/1082801/diff/24001/25011 File user/src/com/google/gwt/html5/canvas/client/ImageData.java (right): http://gwt-code-reviews.appspot.com/1082801/diff/24001/25011#newcode29 user/src/com/google/gwt/html5/canvas/client/ImageData.java:29: public class ImageData extends JavaScriptObject { I think you mentioned added convienence methods to get the rgba values at a specified row/column index. I think we should add those methods because the structore of CanvasPixelArray isn't obvious. int getRedPixelAt(int row, int column) { return gwtData()[row * column]; } int getBluePixelAt(int row, int column) { return gwtData()[row * column + 1]; } ... http://gwt-code-reviews.appspot.com/1082801/diff/24001/25014 File user/src/com/google/gwt/html5/canvas/client/MediaElement.java (right): http://gwt-code-reviews.appspot.com/1082801/diff/24001/25014#newcode61 user/src/com/google/gwt/html5/canvas/client/MediaElement.java:61: return (this.error == null) ? 0 : this.error; Still need to fix this return value. return this.error || null; http://gwt-code-reviews.appspot.com/1082801/diff/24001/25014#newcode85 user/src/com/google/gwt/html5/canvas/client/MediaElement.java:85: return this.getAttribute('src'); return this.src? http://gwt-code-reviews.appspot.com/1082801/diff/24001/25014#newcode97 user/src/com/google/gwt/html5/canvas/client/MediaElement.java:97: return !!(elem.getAttribute('controls')); elem is not defined, and you don't need getAttribute(). return !!this.controls; http://gwt-code-reviews.appspot.com/1082801/diff/24001/25014#newcode117 user/src/com/google/gwt/html5/canvas/client/MediaElement.java:117: return media.seeking; return this.seeking http://gwt-code-reviews.appspot.com/1082801/diff/24001/25014#newcode141 user/src/com/google/gwt/html5/canvas/client/MediaElement.java:141: setBooleanAttr("controls", controls); Why do we set the other booleans but remove this one? Can we just do this.controls = controls? http://gwt-code-reviews.appspot.com/1082801/diff/24001/25014#newcode165 user/src/com/google/gwt/html5/canvas/client/MediaElement.java:165: this.setAttribute('src', url); this.src = url; http://gwt-code-reviews.appspot.com/1082801/diff/24001/25015 File user/src/com/google/gwt/html5/canvas/client/MediaError.java (right): http://gwt-code-reviews.appspot.com/1082801/diff/24001/25015#newcode16 user/src/com/google/gwt/html5/canvas/client/MediaError.java:16: /* Whats up with the double copyright? http://gwt-code-reviews.appspot.com/1082801/diff/24001/25017 File user/src/com/google/gwt/html5/canvas/client/TimeRanges.java (right): http://gwt-code-reviews.appspot.com/1082801/diff/24001/25017#newcode17 user/src/com/google/gwt/html5/canvas/client/TimeRanges.java:17: * Copyright 2009 Mark Renouf Double copyright? http://gwt-code-reviews.appspot.com/1082801/diff/24001/25018 File user/src/com/google/gwt/html5/client/IsSupported.java (right): http://gwt-code-reviews.appspot.com/1082801/diff/24001/25018#newcode26 user/src/com/google/gwt/html5/client/IsSupported.java:26: * @return true if the class is supported. Add a newline before @return. http://gwt-code-reviews.appspot.com/1082801/diff/24001/25020 File user/test/com/google/gwt/html5/canvas/client/CanvasTest.java (right): http://gwt-code-reviews.appspot.com/1082801/diff/24001/25020#newcode40 user/test/com/google/gwt/html5/canvas/client/CanvasTest.java:40: @DoNotRunWith(Platform.HtmlUnitUnknown) +1 None of these tests will work in HtmlUnit if it doesn't support Canvas. http://gwt-code-reviews.appspot.com/1082801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
