Disallow object-valued and any-valued attributes for JS-implemented WebIDL

RESOLVED WONTFIX

Status

()

defect
RESOLVED WONTFIX
6 years ago
5 months ago

People

(Reporter: bholley, Assigned: mccr8)

Tracking

(Blocks 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

smaug points out that the cloning we want to do in bug 923904 violates the semantics of attributes, whereby the value of the attribute should not change on each access (i.e. we should have |foo.attr === foo.attr|).

We've agreed that we should simply disallow implemented such interfaces (which include CustomEvent) in JS. Andrew is going to take the fix here.
Summary: Disallow object-valued and any-valued attributes for WebIDL → Disallow object-valued and any-valued attributes for JS-implemented WebIDL
Yeah, at least for now we should not support implementing such interfaces in JS, and looks like
no JSImplemented interface has such attributes, luckily. 
We can remove this restriction if we figure out good ways to implement it, or
add some annotation to make it clear that an attribute never returns the same object
(which is kind odd).
Looks like I missed some cases. We do have some such attributes in JSImplemented interfaces :(
(In reply to Olli Pettay [:smaug] from comment #2)
> Looks like I missed some cases. We do have some such attributes in
> JSImplemented interfaces :(

Which ones?
★ ~/moz/mozilla-central/dom/webidl $ grep object *.webidl | grep attribute
CanvasRenderingContext2D.webidl:  attribute object mozCurrentTransform; // [ m11, m12, m21, m22, dx, dy ], i.e. row major
CanvasRenderingContext2D.webidl:  attribute object mozCurrentTransformInverse;
Contacts.webidl:  attribute object?    type; // DOMString[]
Contacts.webidl:  attribute object?    type; // DOMString[]
Contacts.webidl:           attribute object?      photo;
Contacts.webidl:           attribute object?      adr;
Contacts.webidl:           attribute object?      email;
Contacts.webidl:           attribute object?      url;
Contacts.webidl:           attribute object?      impp;
Contacts.webidl:           attribute object?      tel;
Contacts.webidl:           attribute object?      name;
Contacts.webidl:           attribute object?      honorificPrefix;
Contacts.webidl:           attribute object?      givenName;
Contacts.webidl:           attribute object?      additionalName;
Contacts.webidl:           attribute object?      familyName;
Contacts.webidl:           attribute object?      honorificSuffix;
Contacts.webidl:           attribute object?      nickname;
Contacts.webidl:           attribute object?      category;
Contacts.webidl:           attribute object?      org;
Contacts.webidl:           attribute object?      jobTitle;
Contacts.webidl:           attribute object?      note;
Contacts.webidl:           attribute object?      key;
Document.webidl:  // special event handler IDL attributes that only apply to Document objects
HTMLAppletElement.webidl:           attribute DOMString _object;
HTMLDocument.webidl:  readonly attribute object all;
IDBDatabase.webidl:    readonly    attribute DOMStringList      objectStoreNames;
IDBIndex.webidl:    readonly    attribute IDBObjectStore objectStore;
IDBTransaction.webidl:    readonly    attribute DOMStringList objectStoreNames;
RTCPeerConnection.webidl:  readonly attribute object localStreams;
RTCPeerConnection.webidl:  readonly attribute object remoteStreams;
★ ~/moz/mozilla-central/dom/webidl $
(In reply to Jan-Ivar Bruaroey [:jib] from comment #4)
> ★ ~/moz/mozilla-central/dom/webidl $ grep object *.webidl | grep attribute

We need to filter by the ones that are JS-implemented.
I threw together a patch, and it turns out this is used in a few places.

object attributes are used in ContactAddress, ContactField, mozContact and mozRTCPeerConnection.  For mozRTCPeerConnection, it is those obsoleted fields that are going to be removed in bug 929530.  Reuben says that he needed to use object for contacts because of the lack of arrays.

any is used in DOMMMIError.  I'm not sure what that is exactly, but it is a very small interface.  Also in the interface TestJSImplInterface, for the attribute jsonifierShouldSkipThis, which presumably could just be commented out.

It would be nice to add a test for this, but I'm not sure how that would work.
Posted patch hacky checkerSplinter Review
This only checks getters (checking setters doesn't add any more as you'd expect), and doesn't throw, but just prints out a message, so you can see everything that fails.

It might be nicer to check for this earlier, in the parser or some such, and then just assert here that the types are okay, but maybe it doesn't matter.
Depends on: 929701
OS: Mac OS X → All
Hardware: x86 → All
Depends on: 929530
Depends on: 939977
We decided to not do this, and did bug 1036214 instead.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WONTFIX
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.