RESOLVED FIXED43254
Expand SVG Attribute Macros
https://bugs.webkit.org/show_bug.cgi?id=43254
Summary Expand SVG Attribute Macros
Fady Samuel
Reported 2010-07-30 08:56:05 PDT
SVG Attributes are currently macros. This often makes debugging SVG a nightmare as we can't trace through macros.
Attachments
SVG Attribute macros expanded (17.92 KB, patch)
2010-07-30 09:00 PDT, Fady Samuel
dglazkov: review-
dglazkov: commit-queue-
SVG Attribute Macros Expanded (24.81 KB, patch)
2010-07-30 09:41 PDT, Fady Samuel
dglazkov: review-
dglazkov: commit-queue-
SVG Attribute Macros Expanded (18.66 KB, patch)
2010-07-30 10:30 PDT, Fady Samuel
no flags
Fady Samuel
Comment 1 2010-07-30 09:00:55 PDT
Created attachment 63065 [details] SVG Attribute macros expanded
Dimitri Glazkov (Google)
Comment 2 2010-07-30 09:35:49 PDT
Comment on attachment 63065 [details] SVG Attribute macros expanded It's missing a ChangeLog. Otherwise, looks good.
Fady Samuel
Comment 3 2010-07-30 09:41:24 PDT
Created attachment 63067 [details] SVG Attribute Macros Expanded
Dimitri Glazkov (Google)
Comment 4 2010-07-30 09:57:33 PDT
Comment on attachment 63067 [details] SVG Attribute Macros Expanded I'll pick on you, but only because you're new here! :P WebCore/ChangeLog:10 + No new tests. Why no new test? Usually a simple explanation is provided, like: "No change in behavior, so no new tests." WebCore/ChangeLog:130 + * rendering/style/SVGRenderStyleDefs.h: This long list of changes is not very useful. Typically, the author of the ChangeLog takes this produced template and makes something useful out of it, like: rendering/style/SVGRenderStyle.h: Expanded and removed references to <list of macros> macros. rendering/style/SVGRenderStyleDefs.h: Removed definitons for expanded macros. Then you can zap the rest of the list in the template.
Fady Samuel
Comment 5 2010-07-30 10:30:37 PDT
Created attachment 63076 [details] SVG Attribute Macros Expanded Made the change log nicer. Thanks dglazkov for your patience with my n00bness at writing useful changelogs.:)
Dimitri Glazkov (Google)
Comment 6 2010-07-30 10:42:14 PDT
Comment on attachment 63076 [details] SVG Attribute Macros Expanded ok.
Nikolas Zimmermann
Comment 7 2010-07-30 11:26:21 PDT
Thanks Fady, good start at cleaning up SVGRenderStyle & friends.
WebKit Commit Bot
Comment 8 2010-07-30 12:06:36 PDT
Comment on attachment 63076 [details] SVG Attribute Macros Expanded Clearing flags on attachment: 63076 Committed r64367: <http://trac.webkit.org/changeset/64367>
WebKit Commit Bot
Comment 9 2010-07-30 12:06:41 PDT
All reviewed patches have been landed. Closing bug.
Eric Seidel (no email)
Comment 10 2010-07-30 14:03:31 PDT
Comment on attachment 63076 [details] SVG Attribute Macros Expanded WebCore/ChangeLog:1 + 2010-07-30 fsamuel@chromium.org <fsamuel@chromium.org> This should be your Full Name <email>, no email twice. :) WebCore/rendering/style/SVGRenderStyle.h:  + SVG_RS_DEFINE_ATTRIBUTE(EAlignmentBaseline, AlignmentBaseline, alignmentBaseline, AB_AUTO) I'm not sure why expanding these is better... but OK.
Fady Samuel
Comment 11 2010-07-30 14:09:14 PDT
(In reply to comment #10) > (From update of attachment 63076 [details]) > WebCore/ChangeLog:1 > + 2010-07-30 fsamuel@chromium.org <fsamuel@chromium.org> > This should be your Full Name <email>, no email twice. :) > > WebCore/rendering/style/SVGRenderStyle.h:  > + SVG_RS_DEFINE_ATTRIBUTE(EAlignmentBaseline, AlignmentBaseline, alignmentBaseline, AB_AUTO) > I'm not sure why expanding these is better... but OK. Made debugging a snap now. Nikolas Zimmermann suggested I make this change in: https://bugs.webkit.org/show_bug.cgi?id=43254
Nikolas Zimmermann
Comment 12 2010-07-30 14:09:29 PDT
(In reply to comment #10) > (From update of attachment 63076 [details]) > WebCore/ChangeLog:1 > + 2010-07-30 fsamuel@chromium.org <fsamuel@chromium.org> > This should be your Full Name <email>, no email twice. :) > > WebCore/rendering/style/SVGRenderStyle.h:  > + SVG_RS_DEFINE_ATTRIBUTE(EAlignmentBaseline, AlignmentBaseline, alignmentBaseline, AB_AUTO) > I'm not sure why expanding these is better... but OK. It was your request, a long time ago, you even added the FIXME :-)
Note You need to log in before you can comment on or make changes to this bug.