In LayoutTests/fast/canvas/webgl/, gl-object-get-calls and bad-arguments-test.html were broken because several cases of returning empty string or undefined were changed to null.
Created attachment 72885 [details] Patch
Comment on attachment 72885 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=72885&action=review > LayoutTests/fast/canvas/webgl/gl-object-get-calls.html:43 > +shouldBeTrue('gl.getAttachedShaders(null) == undefined'); I don't think this should ever return undefined. See https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/doc/spec/WebGL-spec.html#5.14.3 . Why do we not use shouldBeNull here? > WebCore/html/canvas/WebGLRenderingContext.cpp:1633 > + return ""; How are these return values different? Does one translate to null at the JS level and one to an empty string?
Comment on attachment 72885 [details] Patch In the interest of un-breaking the tree I'm going to r+ this patch because it looks like the author is out for the rest of the day. However I'm leaving the bug open because I think a revision is necessary.
(In reply to comment #2) > (From update of attachment 72885 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=72885&action=review > > > LayoutTests/fast/canvas/webgl/gl-object-get-calls.html:43 > > +shouldBeTrue('gl.getAttachedShaders(null) == undefined'); > > I don't think this should ever return undefined. See https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/doc/spec/WebGL-spec.html#5.14.3 . Why do we not use shouldBeNull here? I'm not sure what you're linking to there. Isn't that about lost contexts? In my reading of the spec, both null and undefined are valid return values in the case of an invalid program. > > WebCore/html/canvas/WebGLRenderingContext.cpp:1633 > > + return ""; > > How are these return values different? Does one translate to null at the JS level and one to an empty string? That's exactly it. I could add a comment to this effect.
(In reply to comment #4) > (In reply to comment #2) > > (From update of attachment 72885 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=72885&action=review > > > > > LayoutTests/fast/canvas/webgl/gl-object-get-calls.html:43 > > > +shouldBeTrue('gl.getAttachedShaders(null) == undefined'); > > > > I don't think this should ever return undefined. See https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/doc/spec/WebGL-spec.html#5.14.3 . Why do we not use shouldBeNull here? > > I'm not sure what you're linking to there. Isn't that about lost contexts? "All methods returning nullable values or any return null." > In my reading of the spec, both null and undefined are valid return values in the case of an invalid program. Implementations should not return undefined for WebGLShader[ ] under any circumstances. Where does the spec indicate that this might happen? > > > WebCore/html/canvas/WebGLRenderingContext.cpp:1633 > > > + return ""; > > > > How are these return values different? Does one translate to null at the JS level and one to an empty string? > > That's exactly it. I could add a comment to this effect. Might be helpful.
Committed r71289: <http://trac.webkit.org/changeset/71289>
Accidentally closed this bug when necessarily landing the patch above to un-break the tree. Reopening.
(In reply to comment #7) > Accidentally closed this bug when necessarily landing the patch above to un-break the tree. Reopening. gl-object-get-calls.html is still showing up with an error because of one extra blank line at the end. It ran cleanly, but my git settings stripped that line when I committed locally and uploaded. I will rebaseline this test.