Sorry WebKit for not catching this sooner. Olli is right, they should never have been allowed to be null to begin with.

https://github.com/whatwg/dom/commit/aad67c8f0cd78f3a9157acd481384ae22b5662de
http://dom.spec.whatwg.org/

(In reply to comment #1)
> Sorry WebKit for not catching this sooner. Olli is right, they should never
> have been allowed to be null to begin with.

Luckily WebKit already had these empty for type = 'childList'; I'd hope that there's not much code that looks at added/removed nodes for non-childList records. Just curious, what's the advantage of having those non-null on attributes/characterData records?

No need to null check if you have some generic stuff handling
MutationRecords.

my $.02:

If you are processing every mutation as if it were a childlist, you're doing something wrong and likely to get the wrong answer. It should be an invariant that a childlist mutation record has at least one node in either of added/removed nodes and code which tolerates both being empty is probably wrong.

I think that having an empty array for added/removed in the case of non-childlist changes suggests something misleading about how the API works.

Olli, do you feel strongly about this? It kinda makes sense I guess to make them null in the non-childList case. (Personally I thought we should have had separate record interfaces...)

Reopening so we can reach some kind of conclusion.

I don't feel strongly about this.
But I think returning always an array makes the API more consistent.
addedNodes and removedNodes tell always which nodes where added or removed,
and in cases of non-childList they are empty, since nothing was added or
removed.

Somewhat similar happens for example with
document.querySelectorAll("foo").
If there are no foos, it is empty list, not null which is returned.

I think the point is that the other NodeList accessors are never exposed in a context where the NodeList can only be empty.

This is one of the reasons why I would have preferred distinct MutationRecord interfaces.

Anyway, still not sure what to do.

(In reply to comment #4)
>  It should be an
> invariant that a childlist mutation record has at least one node in either
> of added/removed nodes and code which tolerates both being empty is probably
> wrong.
Why such invariant?

> I think that having an empty array for added/removed in the case of
> non-childlist changes suggests something misleading about how the API works.
I see addedNodes and removedNodes as part of the MutationRecord.
You should always know whether something was added by checking
record.addedNodes.length.

(In reply to comment #10)
> (In reply to comment #4)
> >  It should be an
> > invariant that a childlist mutation record has at least one node in either
> > of added/removed nodes and code which tolerates both being empty is probably
> > wrong.
> Why such invariant?

What Anne said. You're usually the consistency guy =-). You don't find this to be inconsistent with existing APIs?

> 
> > I think that having an empty array for added/removed in the case of
> > non-childlist changes suggests something misleading about how the API works.
> I see addedNodes and removedNodes as part of the MutationRecord.
> You should always know whether something was added by checking
> record.addedNodes.length.

(In reply to comment #11)
> What Anne said. You're usually the consistency guy =-). You don't find this
> to be inconsistent with existing APIs?
> 
No. This is consistent with existing APIs.
You get empty Nodelist all the time from various functions 
(I know, we're dealing attributes here), but not null.


Also, it would be inconsistent to have empty nodelist in case type
is childList, but null in other cases.

(In reply to comment #12)
> (In reply to comment #11)
> > What Anne said. You're usually the consistency guy =-). You don't find this
> > to be inconsistent with existing APIs?
> > 
> No. This is consistent with existing APIs.
> You get empty Nodelist all the time from various functions 
> (I know, we're dealing attributes here), but not null.
> 
> 
> Also, it would be inconsistent to have empty nodelist in case type
> is childList, but null in other cases.

I don't have any further defense for my position. I suppose it's stylistic. My $.02 is that null for the case of non-childlist is correct. I defer to Anne to make the call. FWIW, This doesn't strike me as "make-or-break" either way.

Resolving as FIXED per comment 1 meaning they are not nullable. Rationale is that when e.g. logging everything to a console obj.addedNodes.length would fail.

Argh. I just noticed that every other member does use the nullable pattern. E.g. either a string or null (rather than simply empty string). Olli?

What has that to do with this bug.
null is ok for string attributes IMO, and even needed in cases like
oldValue, where the oldValue might be "".

Meh.

Ok. Just to be clear (because it looks like WebKit needs to change), the decision here is that addedNodes and removedNodes must be empty NodeLists in the case of non-childList mutation type?

Yes.

(as they are also in case of childList when no nodes are added or removed.)