All other toV8(impl) return null when impl is 0.
Created attachment 172326 [details] Patch
jsbell, alecflett: would you check if this patch doesn't break compatibility?
Comment on attachment 172326 [details] Patch Please wait to hear from jsbell or alecflett before landing.
Returning undefined in that case is intentional, see: https://bugs.webkit.org/show_bug.cgi?id=96401 ...which contains the spec reference.
(In reply to comment #4) > Returning undefined in that case is intentional, see: > > https://bugs.webkit.org/show_bug.cgi?id=96401 > > ...which contains the spec reference. ok, then I'll add a comment about it. (The spec sounds strange to me though.)
Created attachment 172336 [details] patch for landing
Comment on attachment 172336 [details] patch for landing This probably isn't the best way to implement this requirement. Instead, the attributes that can return undefined should do so explicitly rather than relying upon toJS/toV8 to do something wonky.
Comment on attachment 172336 [details] patch for landing Makes sense. Will fix it.
Comment on attachment 172336 [details] patch for landing Clearing flags on attachment: 172336 Committed r133483: <http://trac.webkit.org/changeset/133483>
All reviewed patches have been landed. Closing bug.
(In reply to comment #7) > (From update of attachment 172336 [details]) > This probably isn't the best way to implement this requirement. Instead, the attributes that can return undefined should do so explicitly rather than relying upon toJS/toV8 to do something wonky. At first I was thinking about introducing a new IDL attribute [TreatReturnedNullAs=Undefined]. However, as far as I scanned custom bindings, DOM attributes benefited by the IDL attribute are only IDBKeyRange.upper and IDBKeyRange.lower. Maybe it would make sense to just write custom getters for IDBKeyRange.upper and IDBKeyRange.lower?
> At first I was thinking about introducing a new IDL attribute [TreatReturnedNullAs=Undefined]. However, as far as I scanned custom bindings, DOM attributes benefited by the IDL attribute are only IDBKeyRange.upper and IDBKeyRange.lower. Maybe it would make sense to just write custom getters for IDBKeyRange.upper and IDBKeyRange.lower? We should look in WebIDL, but I think technically the right way to handle this is by adding a ? after the type in the IDL file.
(In reply to comment #12) > We should look in WebIDL, but I think technically the right way to handle this is by adding a ? after the type in the IDL file. The situation looks a bit different. 'IDBKey?' means that an IDBKey object and null are valid values. (http://dev.w3.org/2006/webapi/WebIDL/#idl-nullable-type) On the other hand, in our situation we want to return undefined.
Does that mean the behavior of this interface cannot be represented in WebIDL?
(In reply to comment #14) > Does that mean the behavior of this interface cannot be represented in WebIDL? In my understanding, we cannot. (If upper and lower are speced to return null, it is conformed to the WebIDL. In that sense, the spec sounds strange to me.) It is possible to represent the behavior in the WebKit IDL by introducing [TreatReturnedNullAs=Undefined], but the IDL attribute just benefits only IDBKeyRange.upper and IDBKeyRannge.lower. (Also, if we try to support [TreatReturnedNullAs=*] for all types, it would mess up JSValueToNative() in CodeGenerator*.pm.)
For the time being, I'd recommend going with custom code. I don't fully understand why these attributes return undefined. Is there a strong reason for that from the IDB side? If not, we might want to change the spec to better align with the rest of the platform.
(In reply to comment #16) > I don't fully understand why these attributes return undefined. Is there a strong reason for that from the IDB side? If not, we might want to change the spec to better align with the rest of the platform. Agreed. jsbell, alecflett: Do you have any idea?
That sounds totally reasonable to me, but I don't know enough of the background - I defer to jsbell@ here but I expect we'll take it up on the list..
I don't think there's any particular reason to prefer null vs. undefined for these attributes in the spec. Neither are "valid" values for a key. As alecflett@ says, this will come down to list consensus. As far as implementing it, I have webkit.org/b/97375 filed to propose removing all of the custom binding logic around IDBKey, since IDBKey is a WebKit concept not an IDB concept. I was thinking use any/ScriptValue (IDL/C++) and do IDBKey<->ScriptValue conversion in the IDB code itself by calls into the binding utilities. Not sure that's the right direction to go, but it would remove these special cases.
(In reply to comment #13) > The situation looks a bit different. 'IDBKey?' means that an IDBKey object and null are valid values. (http://dev.w3.org/2006/webapi/WebIDL/#idl-nullable-type) On the other hand, in our situation we want to return undefined. And just to re-iterate - the IDB spec doesn't define an IDBKey interface in the IDL; that's entirely a WebKit creation. Wherever WebKitIDLs use IDBKey it is |any| in the IDB spec.
> And just to re-iterate - the IDB spec doesn't define an IDBKey interface in the IDL; that's entirely a WebKit creation. Wherever WebKitIDLs use IDBKey it is |any| in the IDB spec. Is there a particular reason IDB wants to return undefined here rather than null?
(In reply to comment #21) > > Is there a particular reason IDB wants to return undefined here rather than null? No, I don't think so. (I think the IDB spec in general went a little crazy with "undefined"; I seem to recall this was done across the spec at some point about a year ago but I don't recall the rationale or history.)
It might be better if we changed to spec to make these attributes behave like most other attributes in the platform. I don't know how locked down the current spec is, however.