"If either options' attributeOldValue or attributeFilter is present and options' attributes is omitted, set options' attributes to true. "
should also check that attributeFilter has at least one value.

Note that this means that if we add the ability to observe more things in the future, like shadow-DOM stuff, that would mean that

MutationObserver.observe(node, { newFeature: true })

would throw in downlevel browsers, whereas

MutationObserver.observe(node, { newFeature: true, childList: true })

would not throw. This seems somewhat inconsistent and could force authors to wrap try/catches around their code just to suppress exceptions.

Well, MutationObserver.observe(node, { newFeature: true }) certainly should throw if that newFeature isn't supported.

Do you also think that

MutationObserver.observe(node, { newFeature: true, childList: true })

should throw if newFeature is not supported?

Oh, I see.

Yeah, that newFeature shouldn't cause throwing, unless
having newFeature: true and childList: true doesn't make sense in the implementations which support newFeature.

That is similar to 
observe(node, {  attributeOldValue: true, attributes: false}); which throws.

So, {} should throw, since it doesn't cause any node to be observed in anyway.

So do you think that

MutationObserver.observe(node, { newFeature: true })

should throw in current browsers? Or is it specifically *only* empty objects that should cause an exception?

if observe(foo, { ... something...}) doesn't end up doing anything (not observe any changes), that should throw. Scripts should know asap that something is going wrong, not wait later to figure out that the expected MutationRecords won't be there.

(In reply to Olli Pettay from comment #0)
> This was the behavior at some point, but the spec seems to have changed.

[citation needed]


(In reply to Olli Pettay from comment #1)
> should also check that attributeFilter has at least one value.

No, I recall we decided that checking for presence was what we wanted here.

(In reply to Anne from comment #8)
> (In reply to Olli Pettay from comment #0)
> > This was the behavior at some point, but the spec seems to have changed.
> 
> [citation needed]
Gecko's current behavior follows an older version of the spec.


> (In reply to Olli Pettay from comment #1)
> > should also check that attributeFilter has at least one value.
> 
> No, I recall we decided that checking for presence was what we wanted here.

[citation needed] ;)

(In reply to Olli Pettay from comment #7)
> if observe(foo, { ... something...}) doesn't end up doing anything (not
> observe any changes), that should throw. Scripts should know asap that
> something is going wrong, not wait later to figure out that the expected
> MutationRecords won't be there.

You seem to consider "observing fewer things than expected" to be different from "observing nothing". Is that true?

I.e. in both of

MutationObserver.observe(node, { newFeature: true })
MutationObserver.observe(node, { newFeature: true, childList: true })

the newFeature observations won't be fired in a downlevel browser. Are you saying that's worse in the first case than in the second case?

That is mostly hypothetical, and if new features is to be added, we should make
sure that backwards compatibility is sane.

Is there any reason to not throw early if observe() doesn't do anything sane?
(and it used to do that)

Why would newFeature be handled differently to existing properties where we throw in some cases.

Other option is to never throw from observe() (but that would be just a bad API).
Either we throw in all the cases observe() doesn't do anything sane, or we never throw.
No special cases. Consistency, pretty please.

(In reply to Olli Pettay from comment #11)
> Is there any reason to not throw early if observe() doesn't do anything sane?

The reason is that "doesn't do anything sane" is going to vary from use-case to use-case. I.e. in some cases it might be quite sane to simply not receive certain notifications if the browser doesn't support them, in others not.

For example if we add notifications for Shadow DOM, then not firing those in browsers that doesn't support Shadow DOM might be just fine. In such browsers the website might use a Shadow DOM polyfill which provides its own notification mechanism.

The other reason is that we can't really be consistent. I.e. we can't make both of these throw, so it might be more consistent to make neither throw:

MutationObserver.observe(node, { newFeature: true })
MutationObserver.observe(node, { newFeature: true, childList: true })

The reason we can't make both throw is that JS doesn't provide a good way to check that "no unknown properties are provided". Things like non-enumerable properties, properties on prototype objects, and proxies simply make checking for lack of unknown properties too hard/complex.

We're not adding any shadow DOM specific options atm. That is hypothetical still
(observing changes in Shadow DOM would make its encapsulation weaker and weaker, though since it pretty much doesn't any any encapsulation, perhaps that doesn't really matter.)

If one is using options which don't cause any observing would be a 
bug in application logic, and in such case there should be an exception thrown as early as possible.

(In reply to Jonas Sicking from comment #13)
> (In reply to Olli Pettay from comment #11)

> The other reason is that we can't really be consistent. I.e. we can't make
> both of these throw, so it might be more consistent to make neither throw:
> 
> MutationObserver.observe(node, { newFeature: true })
> MutationObserver.observe(node, { newFeature: true, childList: true })
I'm not saying they both should throw. The latter is perfectly valid.

This is no different to
XHR.open("foobar", "http://www.w3.org");
Maybe in the future we'll support "foobar" method, but for now open() throws if invalid method name is passed to it as the first parameter.

That seems like a bad example. It shouldn't throw, and I don't think it does.

"If method is not a method, throw a SyntaxError exception."

Right, and "foobar" is a method per that definition.

Anyway, I'm not sure why we'd start throwing here. Is that what any implementation does?

Make it " foo bar something which doesn't look like the current token in http1" ;)


Gecko throws in this case
var mo = new MutationObserver(function() {});
mo.observe(document, {});
since the spec used to require throwing in that case.

http://hg.mozilla.org/mozilla-central/annotate/ae4d9b4ff2ee/content/base/src/nsDOMMutationObserver.cpp#l456

And I don't know why the spec removed throwing in that case but kept throwing
in some other cases.

You are correct. Originally we had

https://github.com/whatwg/dom/commit/5527027d71d72293f1e5cbafbf5e938f026a3b50

which got changed in

https://github.com/whatwg/dom/commit/a770b11981f4c147f98753cf726e44b14d17affc

based on discussion in bug 23189.

And https://www.w3.org/Bugs/Public/show_bug.cgi?id=23189#c3

* If none of options' childList, attributes, and characterData is true, throw a TypeError.

Something else what committed to the spec.

https://github.com/whatwg/dom/commit/4bf29a4ae4a1e5e4f1a5249f5fc7aa40d37b3f51

Thanks Olli!