[RaisesException] extended attribute was added in Bug 117350 for Web IDL operations. [ConstructorRaisesException] was the pre-existing equivalent for interface constructors. It does not make sense to have 2 Web IDL extended attributes to do the same thing so we should consider replacing [ConstructorRaisesException] with [RaisesException] as blink did.
Created attachment 204148 [details] Patch
Comment on attachment 204148 [details] Patch ok
Comment on attachment 204148 [details] Patch Clearing flags on attachment: 204148 Committed r151378: <http://trac.webkit.org/changeset/151378>
All reviewed patches have been landed. Closing bug.
This change seems strange. Now we label an entire interface “RaisesException” and to me it’s not obvious what that means.
(In reply to comment #5) > This change seems strange. Now we label an entire interface “RaisesException” and to me it’s not obvious what that means. I see. I was trying to reduce the number of WebKit-specific IDL attributes (and be consistent with Blink IDL). However, I see your point that we do loose some readability in this case (and people would likely have to refer to the WebKit IDL wiki). It did not disturb me much because a similar thing was already done for: http://trac.webkit.org/wiki/WebKitIDL#CallWith If you specify [CallWith] on an interface, it applies only to the constructor, not for all attributes / operations of the interface. The attribute could have been called [CallConstructorWith] for clarity but [CallWith] is used (probably for consistency with operations / attributes). Having a limited number of Webkit-specific IDL attributes also has benefits. I don't have strong feelings either way (only slight preference was keeping the patch in). Therefore, if you prefer to keep [ConstructorRaisesException], then I don't mind reverting this patch. However, I would then propose that we introduce [CallConstructorWith] to follow the same pattern.
I agree that CallConstructorWith is clearer than CallWith. I don’t agree that same attribute name used as an interface attribute and an operation attribute with subtly different meanings counts as a smaller number of attributes. We are optimizing a number, number of distinct attribute names, that is not a helpful metric. I agree that minimizing the number of attributes is important. Although, as an aside, I think it is more important to minimize the degree to which class names and behavior are hardcoded into the bindings generation scripts than to minimize the number of attributes. But I believe that reusing the same name for multiple subtly different purposes is a false economy.
(In reply to comment #7) > I agree that CallConstructorWith is clearer than CallWith. > > I don’t agree that same attribute name used as an interface attribute and an operation attribute with subtly different meanings counts as a smaller number of attributes. > > We are optimizing a number, number of distinct attribute names, that is not a helpful metric. > > I agree that minimizing the number of attributes is important. Although, as an aside, I think it is more important to minimize the degree to which class names and behavior are hardcoded into the bindings generation scripts than to minimize the number of attributes. > > But I believe that reusing the same name for multiple subtly different purposes is a false economy. Ok, I'm not disagreeing with you. I will roll this out soon then and likely propose a small patch to introduce CallConstructorWith.
> Ok, I'm not disagreeing with you. I will roll this out soon then and likely propose a small patch to introduce CallConstructorWith. Bikeshedding: [ConstructorCallWith] might be slightly better, since we might want to prefix all constructor related IDL attributes with "Constructor". Historically, previously [CallWith] was [ConstructorCallWith]. I replaced [ConstructorCallWith] with [CallWith] at some point.
Reverted r151378 for reason: Decreased IDL readability a bit Committed r151487: <http://trac.webkit.org/changeset/151487>
(In reply to comment #9) > > Ok, I'm not disagreeing with you. I will roll this out soon then and likely propose a small patch to introduce CallConstructorWith. > > Bikeshedding: [ConstructorCallWith] might be slightly better, since we might want to prefix all constructor related IDL attributes with "Constructor". > > Historically, previously [CallWith] was [ConstructorCallWith]. I replaced [ConstructorCallWith] with [CallWith] at some point. Makes sense Kentaro. I'll use that instead then, thanks.
Thanks guys. Hope my wording didn’t come across as harsh. I may have prioritized clarity over politeness a bit!
(In reply to comment #12) > Thanks guys. Hope my wording didn’t come across as harsh. I may have prioritized clarity over politeness a bit! Not at all :) Thanks for the feedback.