Bug 48962 – Broken WebGL tests due to r71274

• Component: WebGL
• Status: RESOLVED FIXED

[Adrienne Walker]

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.

[Adrienne Walker]

Created attachment 72885 [details]
Patch

[Kenneth Russell]

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?

[Kenneth Russell]

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.

[Adrienne Walker]

(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.

[Kenneth Russell]

(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.

[Kenneth Russell]

Committed r71289: <http://trac.webkit.org/changeset/71289>

[Kenneth Russell]

Accidentally closed this bug when necessarily landing the patch above to un-break the tree. Reopening.

[Adrienne Walker]

(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.