http://gwt-code-reviews.appspot.com/1082801/diff/78001/79009 File user/src/com/google/gwt/canvas/dom/client/CssColor.java (right):
http://gwt-code-reviews.appspot.com/1082801/diff/78001/79009#newcode27 user/src/com/google/gwt/canvas/dom/client/CssColor.java:27: * To handle devmode we must wrap JSO strings in an array. Therefore, when in devmode, CssColor is On 2010/11/18 21:54:34, jlabanca wrote:
Wrap in a paragraph <p>
Done. http://gwt-code-reviews.appspot.com/1082801/diff/78001/79009#newcode51 user/src/com/google/gwt/canvas/dom/client/CssColor.java:51: * wrap the String in an array if necessary. On 2010/11/18 21:54:34, jlabanca wrote:
Move this comment inline into the method. You already mention the
array thing in
the class JavaDoc.
Done. http://gwt-code-reviews.appspot.com/1082801/diff/78001/79009#newcode66 user/src/com/google/gwt/canvas/dom/client/CssColor.java:66: * unwrap the String if necessary. On 2010/11/18 21:54:34, jlabanca wrote:
Move this comment inline into the method. You already mention the
array thing in
the class JavaDoc.
Done. http://gwt-code-reviews.appspot.com/1082801/diff/78001/79010 File user/src/com/google/gwt/canvas/dom/client/FillStrokeStyle.java (right): http://gwt-code-reviews.appspot.com/1082801/diff/78001/79010#newcode39 user/src/com/google/gwt/canvas/dom/client/FillStrokeStyle.java:39: * check its type when in devmode (when isScript == false.) @see CssColor On 2010/11/18 21:54:34, jlabanca wrote:
Move this comment inline into the method. Its just an implementation
detail. Done. http://gwt-code-reviews.appspot.com/1082801/diff/78001/79014 File user/src/com/google/gwt/dom/client/Document.java (right): http://gwt-code-reviews.appspot.com/1082801/diff/78001/79014#newcode17 user/src/com/google/gwt/dom/client/Document.java:17: On 2010/11/18 21:54:34, jlabanca wrote:
extra newline
Done. http://gwt-code-reviews.appspot.com/1082801/diff/78001/79016 File user/src/com/google/gwt/user/client/IsSupported.java (right): http://gwt-code-reviews.appspot.com/1082801/diff/78001/79016#newcode23 user/src/com/google/gwt/user/client/IsSupported.java:23: * "boolean isSupported()" that checks whether the canvas method is supported at runtime. On 2010/11/18 21:54:34, jlabanca wrote:
Use <code>boolean isSupported()</code> instead of wrapping in quotes.
Replace "canvas" with "feature".
Done. http://gwt-code-reviews.appspot.com/1082801/diff/78001/79019 File user/test/com/google/gwt/canvas/dom/client/Context2dTest.java (right): http://gwt-code-reviews.appspot.com/1082801/diff/78001/79019#newcode261 user/test/com/google/gwt/canvas/dom/client/Context2dTest.java:261: ImageData verifyPx = context.getImageData(10, 10, 1, 1); // will fail on FF3.0 On 2010/11/18 21:54:34, jlabanca wrote:
Why does this fail on FF3? In any case, we test on FF3, so you'll
have to do a
runtime check and skip this test for FF3.
There's a bug where it will throw a security exception when using getImageData(). (I documented the other FF3.0 deficiencies in line.) I added the method isGecko19OrBefore() and have gated the FF3 unsupported methods. http://gwt-code-reviews.appspot.com/1082801/diff/78001/79019#newcode271 user/test/com/google/gwt/canvas/dom/client/Context2dTest.java:271: // (testing that values are clamped to 0..255 failed due to FF3.5 not implementing it correctly) On 2010/11/18 21:54:34, jlabanca wrote:
This is the kind of thing we should handle. In the setter(), go
through
com.google.gwt.dom.client.DOMImpl and manually clamp the range in
Mozilla. I relaxed the API a bit so that the clamping isn't enforced because I think enforcing it would require too many checks (two comparisons per pixel.) I added a comment in CanvasPixelArray to reflect this, and removed the test. http://gwt-code-reviews.appspot.com/1082801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
