LGTM if you address comments

Our checkstyles for tests are more lax, but I still think its good to
sort methods by visibility.


http://gwt-code-reviews.appspot.com/1415801/diff/1/user/src/com/google/gwt/media/client/Video.java
File user/src/com/google/gwt/media/client/Video.java (right):

http://gwt-code-reviews.appspot.com/1415801/diff/1/user/src/com/google/gwt/media/client/Video.java#newcode88
user/src/com/google/gwt/media/client/Video.java:88: return
(VideoElement) getMediaElement();
I liked cast() better than a cast if it works.

http://gwt-code-reviews.appspot.com/1415801/diff/1/user/test/com/google/gwt/canvas/client/CanvasTest.java
File user/test/com/google/gwt/canvas/client/CanvasTest.java (right):

http://gwt-code-reviews.appspot.com/1415801/diff/1/user/test/com/google/gwt/canvas/client/CanvasTest.java#newcode41
user/test/com/google/gwt/canvas/client/CanvasTest.java:41: private
native boolean isFirefox35OrLater() /*-{
public, private, and protected methods appear out of order.  Also, you
can make these isXXX() checks static.

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

http://gwt-code-reviews.appspot.com/1415801/diff/1/user/test/com/google/gwt/media/client/AudioTest.java#newcode84
user/test/com/google/gwt/media/client/AudioTest.java:84: public
MediaBase getMedia() {
public method after protected method

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

http://gwt-code-reviews.appspot.com/1415801/diff/1/user/test/com/google/gwt/media/client/MediaTest.java#newcode110
user/test/com/google/gwt/media/client/MediaTest.java:110: // wait an
additional 1000ms, then check that the seek was successful
1000ms => 5000ms

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

http://gwt-code-reviews.appspot.com/1415801/diff/1/user/test/com/google/gwt/media/client/VideoTest.java#newcode71
user/test/com/google/gwt/media/client/VideoTest.java:71:
assertEquals(height + "px", video.getOffsetHeight());
Does this pass?  getOffsetHeight/Width() returns an int, so this test
should always fail.

http://gwt-code-reviews.appspot.com/1415801/

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

Reply via email to