http://gwt-code-reviews.appspot.com/1195801/diff/19001/20001
File user/src/com/google/gwt/canvas/client/package-info.java (right):

http://gwt-code-reviews.appspot.com/1195801/diff/19001/20001#newcode18
user/src/com/google/gwt/canvas/client/package-info.java:18: * Widgets
for HTML Canvas 2D support.
For futureproofing, can this be changed to just "Widgets for HTML Canvas
support."? (Here, and in the other Canvas package-info)

http://gwt-code-reviews.appspot.com/1195801/diff/19001/20004
File user/src/com/google/gwt/dom/client/DOMImplMozilla.java (right):

http://gwt-code-reviews.appspot.com/1195801/diff/19001/20004#newcode42
user/src/com/google/gwt/dom/client/DOMImplMozilla.java:42: * Return true
if using Firefox.
For consistency, could this be renamed to Gecko, since it's for the
gecko permutation (despite this being the Mozilla Impl)?

http://gwt-code-reviews.appspot.com/1195801/diff/19001/20005
File user/src/com/google/gwt/dom/client/Document.java (right):

http://gwt-code-reviews.appspot.com/1195801/diff/19001/20005#newcode17
user/src/com/google/gwt/dom/client/Document.java:17:
Extra space

http://gwt-code-reviews.appspot.com/1195801/diff/19001/20006
File user/src/com/google/gwt/dom/client/MediaElement.java (right):

http://gwt-code-reviews.appspot.com/1195801/diff/19001/20006#newcode488
user/src/com/google/gwt/dom/client/MediaElement.java:488: * Sets the
playback rate.
Possibly add a note about the issue where Chrome requires that it must
be playing first.

http://gwt-code-reviews.appspot.com/1195801/diff/19001/20015
File user/src/com/google/gwt/media/dom/client/package-info.java (right):

http://gwt-code-reviews.appspot.com/1195801/diff/19001/20015#newcode18
user/src/com/google/gwt/media/dom/client/package-info.java:18: * DOM
classes for HTML Audio and Video support.
Possibly add the experimental warning here?

(here, and in other files.)

http://gwt-code-reviews.appspot.com/1195801/diff/19001/20019
File user/test/com/google/gwt/media/client/AudioTest.java (right):

http://gwt-code-reviews.appspot.com/1195801/diff/19001/20019#newcode39
user/test/com/google/gwt/media/client/AudioTest.java:39: final static
String audioFormatOgg = "audio/ogg";
Will these be included in the patch?

http://gwt-code-reviews.appspot.com/1195801/diff/19001/20020
File user/test/com/google/gwt/media/client/MediaTest.java (right):

http://gwt-code-reviews.appspot.com/1195801/diff/19001/20020#newcode207
user/test/com/google/gwt/media/client/MediaTest.java:207: //
element.setPlaybackRate(1.0); // return to 1.0
Hand-wrote this test in javascript and found the media has to be playing
first (why?.. sigh.)
Can you set it to play, then set the playback rate? (at least on Chrome)

http://gwt-code-reviews.appspot.com/1195801/diff/19001/20020#newcode222
user/test/com/google/gwt/media/client/MediaTest.java:222: //        ||
state.equals(MediaElement.PRELOAD_NONE));
Looks like this was only added in Firefox 4.0
(http://hacks.mozilla.org/2010/08/video_preload_attribute).
Can you add a conditional for FF4.0+?

http://gwt-code-reviews.appspot.com/1195801/diff/19001/20021
File user/test/com/google/gwt/media/client/VideoTest.java (right):

http://gwt-code-reviews.appspot.com/1195801/diff/19001/20021#newcode28
user/test/com/google/gwt/media/client/VideoTest.java:28: *  Because
HtmlUnit does not support HTML5, you will need to run these tests
Extra space:
"*  Because..." -> "* Because..."
(here, and in other files.)

http://gwt-code-reviews.appspot.com/1195801/diff/19001/20021#newcode115
user/test/com/google/gwt/media/client/VideoTest.java:115: String
posterUrl = "http://www.google.com/images/logos/ps_logo2.png";;
Can a local image be used instead? Or at least a more stable url?

http://gwt-code-reviews.appspot.com/1195801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to