19463 2012-10-11 09:58:41 +0000 MutationRecord added/removedNodes could return empty array, not null 2012-11-20 18:17:00 +0000 1 1 1 Unclassified WebAppsWG DOM unspecified PC Linux RESOLVED FIXED P2 normal --- 1 bugs annevk adamk mike rafaelw www-dom public-webapps-bugzilla oldest_to_newest 75955 0 bugs 2012-10-11 09:58:41 +0000 This is the behavior in Gecko, and it makes the API, IMO, easier to use and more consistent with other nodelist getter APIs. 75982 1 annevk 2012-10-11 14:22:10 +0000 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/ 76022 2 adamk 2012-10-11 18:39:58 +0000 (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? 76023 3 bugs 2012-10-11 18:48:25 +0000 No need to null check if you have some generic stuff handling MutationRecords. 76161 4 rafaelw 2012-10-12 20:13:42 +0000 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. 76162 5 annevk 2012-10-12 20:17:56 +0000 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...) 76163 6 annevk 2012-10-12 20:18:35 +0000 Reopening so we can reach some kind of conclusion. 76211 7 bugs 2012-10-13 13:56:06 +0000 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. 76212 8 bugs 2012-10-13 14:00:30 +0000 Somewhat similar happens for example with document.querySelectorAll("foo"). If there are no foos, it is empty list, not null which is returned. 76481 9 annevk 2012-10-17 09:08:19 +0000 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. 76488 10 bugs 2012-10-17 11:11:41 +0000 (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. 76614 11 rafaelw 2012-10-18 21:20:14 +0000 (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. 76619 12 bugs 2012-10-18 21:39:29 +0000 (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. 76625 13 rafaelw 2012-10-18 22:10:42 +0000 (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. 78044 14 annevk 2012-11-08 14:12:53 +0000 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. 78048 15 annevk 2012-11-08 14:36:07 +0000 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? 78055 16 bugs 2012-11-08 15:38:12 +0000 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 "". 78056 17 annevk 2012-11-08 15:40:55 +0000 Meh. 78545 18 rafaelw 2012-11-20 18:13:48 +0000 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? 78546 19 bugs 2012-11-20 18:16:10 +0000 Yes. 78548 20 bugs 2012-11-20 18:17:00 +0000 (as they are also in case of childList when no nodes are added or removed.)