Bug 91768 – Replaceable attributes should also have readonly

• Component: DOM
• Status: RESOLVED FIXED

[Erik Arvidsson]

DOMWindow.idl (and others) has a bunch of [Replaceable] attributes. These are all marked as readonly [Replaceable] in the specs.

[Erik Arvidsson]

Created attachment 153348 [details]
Patch

[Adam Barth]

Comment on attachment 153348 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=153348&action=review

Does it ever make sense to have a non-readonly [Replaceable] property?  If not, should the code generator error out in that case?

> Source/WebCore/ChangeLog:8
> +        This udpdates the code generators for JSC and V8 to handle this case correctly.

udpdates -> updates

[Erik Arvidsson]

(In reply to comment #2)
> (From update of attachment 153348 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=153348&action=review
> 
> Does it ever make sense to have a non-readonly [Replaceable] property?  If not, should the code generator error out in that case?

Conceptually it could make sense if the ObjC or some other non JS binding want to allow the underlying attribute to be writable.

[Erik Arvidsson]

Created attachment 153355 [details]
Patch

[Adam Barth]

Comment on attachment 153355 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=153355&action=review

r=me assuming this doesn't break ObjC bindings.

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1221
> -        if ($attribute->type =~ /readonly/) {
> +        if ($attribute->type =~ /readonly/ && !$attribute->signature->extendedAttributes->{"Replaceable"}) {

Should we make a helper function that does this instead of copy/pasting it everywhere?

> Source/WebCore/page/DOMWindow.idl:55
> +        readonly attribute [Replaceable] Navigator navigator;

I'm surprised this doesn't break the ObjC bindings.  Does ObjC not have setters for all of these?

[Erik Arvidsson]

Created attachment 153374 [details]
Patch

[Erik Arvidsson]

Created attachment 170930 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 170930 [details]
Patch for landing

Clearing flags on attachment: 170930

Committed r132667: <http://trac.webkit.org/changeset/132667>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.