Bug 65337 - Remove LegacyDefaultOptionalArguments flag from SVG IDL files
Summary: Remove LegacyDefaultOptionalArguments flag from SVG IDL files
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-28 12:27 PDT by Mark Pilgrim (Google)
Modified: 2011-08-02 14:37 PDT (History)
3 users (show)

See Also:


Attachments
Patch (28.40 KB, patch)
2011-07-28 12:28 PDT, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Pilgrim (Google) 2011-07-28 12:27:39 PDT
As discussed in IRC, we are migrating our IDL files away from the interface-level "LegacyDefaultOptionalArguments" flag and onto argument-level [Optional] or [Optional=CallWithDefaultValue] flags. This patch migrates all remaining SVG-related IDL files. It does not change any behavior, i.e. it does not make any arguments required that were previously optional, nor vice-versa.

All existing tests pass.
Comment 1 Mark Pilgrim (Google) 2011-07-28 12:28:35 PDT
Created attachment 102286 [details]
Patch
Comment 2 Nikolas Zimmermann 2011-07-28 12:45:42 PDT
In principle, I'm fine with this patch, as it preserves the existing behaviour.
Though wouldn't it be a good idea to enforce strictness? In the past I've converted our primitives SVGLength/SVGAngle to RequiresAllArguments=Raise, to be more strict.
Comment 3 Adam Barth 2011-07-28 12:49:39 PDT
(In reply to comment #2)
> In principle, I'm fine with this patch, as it preserves the existing behaviour.
> Though wouldn't it be a good idea to enforce strictness? In the past I've converted our primitives SVGLength/SVGAngle to RequiresAllArguments=Raise, to be more strict.

The main question is whether that's going to be a compatibility problem.  In general, I'd like to align with Firefox here as much as possible, but I don't have a sense for how large the compat risk is.
Comment 4 Nikolas Zimmermann 2011-07-28 12:59:05 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > In principle, I'm fine with this patch, as it preserves the existing behaviour.
> > Though wouldn't it be a good idea to enforce strictness? In the past I've converted our primitives SVGLength/SVGAngle to RequiresAllArguments=Raise, to be more strict.
> 
> The main question is whether that's going to be a compatibility problem.  In general, I'd like to align with Firefox here as much as possible, but I don't have a sense for how large the compat risk is.
I highly doubt it, but we won't know unless we try :-)

We've aligned with FF regarding SVGAngle/SVGLength/... all SVG DOM primitives raise exceptions when setting wrong/invalid values (this is in contrary to what SVG 1.1 Second Edition specs - and may change in future).
Comment 5 Mark Pilgrim (Google) 2011-07-28 13:05:49 PDT
Enforcing strictness is a fine idea but really orthogonal to this particular patch. This is a step on the road of allowing us to get rid of the LegacyDefaultOptionalArguments flag and simplify the IDL code generator. I would be happy to be involved in a discussion and/or investigation of whether we can make some arguments required, but that's easy enough to do after this lands. In fact, it will be easier, since you will be able to control it on an argument-by-argument basis.
Comment 6 Nikolas Zimmermann 2011-07-28 13:08:11 PDT
Comment on attachment 102286 [details]
Patch

That argument convinced me, r=me, given no regressions change in behaviour.
Comment 7 WebKit Review Bot 2011-08-02 14:37:05 PDT
Comment on attachment 102286 [details]
Patch

Clearing flags on attachment: 102286

Committed r92237: <http://trac.webkit.org/changeset/92237>
Comment 8 WebKit Review Bot 2011-08-02 14:37:10 PDT
All reviewed patches have been landed.  Closing bug.