Bug 38297

Summary:

Webkit doesn't compile with 3D-canvas enabled and video disabled

Product:

WebKit

Reporter:

Noam Rosenthal <noam>

Component:

WebGL

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
compile fix	darin: review-, darin: commit-queue-
paragraph style fix	levin: review-
style fix to the style fix	none

Noam Rosenthal

Reported 2010-04-28 17:42:56 PDT

JSWebGLRenderingContextCustom.cpp contains video files without an #ifdef guard

Attachments
compile fix (2.71 KB, patch) 2010-04-28 17:44 PDT, Noam Rosenthal	darin: review- darin: commit-queue-	Details Formatted Diff Diff
paragraph style fix (2.84 KB, patch) 2010-04-28 18:57 PDT, Noam Rosenthal	levin: review-	Details Formatted Diff Diff
style fix to the style fix (2.99 KB, patch) 2010-04-29 18:07 PDT, Noam Rosenthal	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Noam Rosenthal

Comment 1 2010-04-28 17:44:37 PDT

Created attachment 54643 [details] compile fix

Darin Adler

Comment 2 2010-04-28 17:52:15 PDT

Comment on attachment 54643 [details] compile fix > #include "HTMLVideoElement.h" > #include "JSHTMLCanvasElement.h" > #include "JSHTMLImageElement.h" > +#if ENABLE(VIDEO) > #include "JSHTMLVideoElement.h" > +#endif > #include "JSImageData.h" > #include "JSWebGLBuffer.h" > #include "JSWebGLFloatArray.h" Conditional includes should go in their own paragraph, after the unconditional ones, not sorted in the include list. Sorry to be so picky about a minor aspect of the patch.

Noam Rosenthal

Comment 3 2010-04-28 18:57:13 PDT

Created attachment 54662 [details] paragraph style fix no problem with style nit-picks! they make the webkit code easier to read...

David Levin

Comment 4 2010-04-29 00:28:04 PDT

Comment on attachment 54662 [details] paragraph style fix > diff --git a/WebCore/bindings/js/JSWebGLRenderingContextCustom.cpp b/WebCore/bindings/js/JSWebGLRenderingContextCustom.cpp > index e336027..9481a0c 100644 > --- a/WebCore/bindings/js/JSWebGLRenderingContextCustom.cpp > +++ b/WebCore/bindings/js/JSWebGLRenderingContextCustom.cpp > @@ -32,10 +32,18 @@ > #include "ExceptionCode.h" > #include "HTMLCanvasElement.h" > #include "HTMLImageElement.h" > + > +#if ENABLE(VIDEO) > #include "HTMLVideoElement.h" > +#endif > + > #include "JSHTMLCanvasElement.h" > #include "JSHTMLImageElement.h" > + > +#if ENABLE(VIDEO) > #include "JSHTMLVideoElement.h" > +#endif > + > #include "JSImageData.h" > #include "JSWebGLBuffer.h" > #include "JSWebGLFloatArray.h" This has the same problem that Darin noted last time. It should look some thing like this (note that the ifdef'ed includes are placed after all others): #include "ExceptionCode.h" #include "HTMLCanvasElement.h" #include "HTMLImageElement.h" #include "JSHTMLCanvasElement.h" #include "JSHTMLImageElement.h" #include "JSImageData.h" #include "JSWebGLBuffer.h" #include "JSWebGLFloatArray.h" #if ENABLE(VIDEO) #include "HTMLVideoElement.h" #include "JSHTMLVideoElement.h" #endif

Noam Rosenthal

Comment 5 2010-04-29 18:07:48 PDT

Created attachment 54766 [details] style fix to the style fix

WebKit Commit Bot

Comment 6 2010-05-02 00:12:45 PDT

Comment on attachment 54766 [details] style fix to the style fix Clearing flags on attachment: 54766 Committed r58642: <http://trac.webkit.org/changeset/58642>

WebKit Commit Bot

Comment 7 2010-05-02 00:12:50 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.