This might not be a bug per se; this is a request to have a simple way of disabling all the SVG features for WebKit/Qt. A similar build logic exist for Gtk builds (see ChangeSet r33926 in configure.ac), this is a request to bring the Qt build up-to-par with Gtk.
Created attachment 28744 [details] Patch to disable all the SVG features for WebKit/Qt if ENABLE_SVG=0
Comment on attachment 28744 [details] Patch to disable all the SVG features for WebKit/Qt if ENABLE_SVG=0 Looks sensible and will work with the below macros. Another way would be to put the ENABLE_SVG_* macros into a block... e.g. contains(DEFINES, ENABLE_SVG=1) { !contains(FOO): DEFINES += ENABLE_FOO=1 } but I would have to check the qmake manual... it would be nice if you could do a follow up.
(In reply to comment #2) > (From update of attachment 28744 [details] [review]) > Looks sensible and will work with the below macros. Another way would be to put > the ENABLE_SVG_* macros into a block... > > e.g. contains(DEFINES, ENABLE_SVG=1) { > !contains(FOO): DEFINES += ENABLE_FOO=1 > } > > but I would have to check the qmake manual... it would be nice if you could do > a follow up. > Thanks for the review ! It seems to me that one of the goals of the build system is to make sure that all the ENABLE_FOO flags are defined (either to 0 or 1) instead of leaving them undefined (as an example ENABLE_WML is set to 0 if it is not defined otherwise). The patch originally proposed maintains this goal, but the suggestion above does not maintain this goal and will leave ENABLE_FOO undefined if ENABLE_SVG is set to 0. This might be a matter of taste, but I think that the original patch is more consistent with the existing code.
(In reply to comment #3) > This might be a matter of taste, but I think that the original patch is more > consistent with the existing code. hehe, you can always add an else { } makes this even more symmetric, minor details though.
Created attachment 28973 [details] Revised patch based on feedback !
Comment on attachment 28973 [details] Revised patch based on feedback ! > +!contains(DEFINES, ENABLE_SVG=0) { > + !contains(DEFINES, ENABLE_SVG=.): DEFINES += ENABLE_SVG=1 this looks a bit weird but should still do what you wanted.
(In reply to comment #6) > (From update of attachment 28973 [details] [review]) > > +!contains(DEFINES, ENABLE_SVG=0) { > > + !contains(DEFINES, ENABLE_SVG=.): DEFINES += ENABLE_SVG=1 > this looks a bit weird but should still do what you wanted. The intention was not to define ENABLE_SVG=1 one more time if it was already defined.
Comment on attachment 28973 [details] Revised patch based on feedback ! This patch has rotted and no longer applies. marking r-
Comment on attachment 28744 [details] Patch to disable all the SVG features for WebKit/Qt if ENABLE_SVG=0 Clearing review flag to remove from commit queue.
Created attachment 29904 [details] Update patch to apply to trunk
Comment on attachment 29904 [details] Update patch to apply to trunk Looks fine.
Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/WebCore.pro Committed r43281