RESOLVED FIXED 24693
Disable all the SVG features for WebKit/Qt if ENABLE_SVG=0
https://bugs.webkit.org/show_bug.cgi?id=24693
Summary Disable all the SVG features for WebKit/Qt if ENABLE_SVG=0
Laszlo Gombos
Reported 2009-03-18 19:07:25 PDT
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.
Attachments
Patch to disable all the SVG features for WebKit/Qt if ENABLE_SVG=0 (1.30 KB, patch)
2009-03-18 19:11 PDT, Laszlo Gombos
no flags
Revised patch based on feedback ! (2.21 KB, patch)
2009-03-26 11:51 PDT, Laszlo Gombos
eric: review-
Update patch to apply to trunk (2.33 KB, patch)
2009-04-30 00:17 PDT, Laszlo Gombos
eric: review+
Laszlo Gombos
Comment 1 2009-03-18 19:11:56 PDT
Created attachment 28744 [details] Patch to disable all the SVG features for WebKit/Qt if ENABLE_SVG=0
Holger Freyther
Comment 2 2009-03-25 00:15:31 PDT
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.
Laszlo Gombos
Comment 3 2009-03-25 12:05:03 PDT
(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.
Holger Freyther
Comment 4 2009-03-25 19:50:43 PDT
(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.
Laszlo Gombos
Comment 5 2009-03-26 11:51:52 PDT
Created attachment 28973 [details] Revised patch based on feedback !
Holger Freyther
Comment 6 2009-04-02 23:16:07 PDT
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.
Laszlo Gombos
Comment 7 2009-04-03 05:37:03 PDT
(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.
Eric Seidel (no email)
Comment 8 2009-04-29 14:45:15 PDT
Comment on attachment 28973 [details] Revised patch based on feedback ! This patch has rotted and no longer applies. marking r-
Eric Seidel (no email)
Comment 9 2009-04-29 14:57:02 PDT
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.
Laszlo Gombos
Comment 10 2009-04-30 00:17:30 PDT
Created attachment 29904 [details] Update patch to apply to trunk
Eric Seidel (no email)
Comment 11 2009-05-03 21:50:19 PDT
Comment on attachment 29904 [details] Update patch to apply to trunk Looks fine.
Eric Seidel (no email)
Comment 12 2009-05-05 22:47:31 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/WebCore.pro Committed r43281
Note You need to log in before you can comment on or make changes to this bug.