WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Revised patch based on feedback !
(2.21 KB, patch)
2009-03-26 11:51 PDT
,
Laszlo Gombos
eric
: review-
Details
Formatted Diff
Diff
Update patch to apply to trunk
(2.33 KB, patch)
2009-04-30 00:17 PDT
,
Laszlo Gombos
eric
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
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.
Top of Page
Format For Printing
XML
Clone This Bug