1129239 - Dictionaries with required properties shouldn't need to be optional

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

Currently we seem to require the last param to be optional if it is a dictionary, 
but that doesn't quite make sense if the dictionary has required properties.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

https://www.w3.org/Bugs/Public/show_bug.cgi?id=27953

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Don't require 'optional' keyword on trailing dictionary arguments if the dictionary has a required member

Assumin the spec changes, this would do the trick

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8558885 [details] [diff] [review]
Don't require 'optional' keyword on trailing dictionary arguments if the dictionary has a required member

Ok, so optional dictionaries with required params is still possible, but not needed.
Should we handle optional dictionaries with required properties somehow differently?

Comment 4

[Boris Zbarsky [:bzbarsky]]

The only principled thing to do there would be to have a distinction between "not passed" and "passed" for dictionaries with required props.

But in what situation would an API actually have different behavior there?  That is, if you want to allow "not passed", why do you have required props?  What do you do in the "not passed" case for those props?

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

hmm, right. Maybe there isn't good use case for that after all.

Comment 6

[Cameron McCormack (:heycam)]

Allowed the optional to be dropped when the dictionary has required members:

https://www.w3.org/Bugs/Public/show_bug.cgi?id=27953

Comment 7

[Boris Zbarsky [:bzbarsky]]

Oh, nice catch about ancestors with required members.  I'll fix the patch here accordingly before landing it.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: With the ancestor dictionary thing

This required me to move a bunch of validation to validate() methods, because otherwise it was happening before the dictionary knew what its parent is.  I considered having complete() on the wrapper type finish() the dictionary, but conceptually putting validation in validate() makes more sense.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Interdiff from previous patch

This mostly moves stuff over to validate() and makes the union code not try to compute things needed for validation in complete(), deferring them to whenever they're asked for instead.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8593779 [details] [diff] [review]
With the ancestor dictionary thing

>+    def isOptional(self):
So rename this to canBeEmpty(self) or some such
ne

>+    def hasOptionalDictionaryType(self):
>+        return (self._dictionaryType is not None and
>+                self._dictionaryType.inner.isOptional())
this too

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/12c1bc15adfc

Comment 12

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/12c1bc15adfc