Using childNodes[ explicit index ] is a bug.

[Rick Waldron]

TL;DR: Use *node.children*, do not use node.childNodes.

After speaking with Julien on IRC this morning, regarding the use of
node.childNodes[ explicit index ], he suggested that I send an email to
list explaining the danger in using this pattern to identify DOM nodes.

There are currently 23 uses of node.childNodes[ explicit index ] in the
Gaia codebase (master), all of which are refactoring hazards/bugs. The
node.childNodes property is a NodeList that contains all child nodes,
regardless of type.

For example, given the following HTML:

<div id="container">
<!-- Put something here -->
<p>lorem ipsum</p>
</div>

The childNodes NodeList of "container" would look like:

[
#text,
<!-- Put something here -->,
#text,
<!-- Updated the UI to use .paragraphs; UI Bug #999 -->,
#text,
<p class="paragraphs">lorem ipsum</p>
]

That's:
0. the space before the comment
1. the comment
2. the space before the <p>
3. the <p>

Given the imaginary app JavaScript:

var container = document.getElementById("container");
container.childNodes[3].innerHTML = "Real text";

If a UI designer were to make changes to the layout in the given HTML,
without altering any of the app JavaScript, to:

<div id="container">
<!-- Put something here -->
<!-- Updated the UI to use .paragraphs; UI Bug #999 -->
<p class="paragraphs">lorem ipsum</p>
</div>


The childNodes NodeList would now look like this:

[
#text,
<!-- Put something here -->,
#text,
<!-- Updated the UI to use .paragraphs; UI Bug #999 -->,
#text,
<p class="paragraphs">lorem ipsum</p>
]


Which means that the app JavaScript:

container.childNodes[3].innerHTML

Is now going to be the newly added comment line.


This can be completely avoided by only using node.children, which is
limited to node of nodeType = 1.


Rick

[Kevin Grandon]

Good point - we should definitely prefer node.children where possible. Two other handy features are:

nextElementSibling instead of nextSibling: https://developer.mozilla.org/en-US/docs/DOM/Element.nextElementSibling
previousElementSibling instead of previousSibling: https://developer.mozilla.org/en-US/docs/DOM/Element.previousElementSibling

Thanks,
Kevin

_______________________________________________
dev-gaia mailing list
dev-...@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-gaia

[Andreas Gal]

Talking about bugs, please also avoid innerHTML if you are just updating text. .textContent is much faster since it bypasses the parser.

Andreas

Sent from mobile.

[Axel Hecht]

On 18.02.13 20:19, Andreas Gal wrote:
> Talking about bugs, please also avoid innerHTML if you are just updating text. .textContent is much faster since it bypasses the parser.

Plus, if you're getting strings from l10n, you should prefer textContent
to reduce the attack surface we have against errors in our
localizations. Only use html if you really need localized markup.

Axel

[Dietrich Ayala]

So much general "best practices" advice on this list lately. It would
be great if someone compiled it all into one doc somewhere.

On Mon, Feb 18, 2013 at 12:53 PM, Axel Hecht <l1...@mozilla.com> wrote:
> On 18.02.13 20:19, Andreas Gal wrote:
>>

>> Talking about bugs, please also avoid innerHTML if you are just updating
>> text. .textContent is much faster since it bypasses the parser.
>
>

> Plus, if you're getting strings from l10n, you should prefer textContent to
> reduce the attack surface we have against errors in our localizations. Only
> use html if you really need localized markup.
>
> Axel
>
>>

[Julien Wajsberg]

Le 18/02/2013 20:09, Kevin Grandon a écrit :
> Good point - we should definitely prefer node.children where possible. Two other handy features are:
>
> nextElementSibling instead of nextSibling: https://developer.mozilla.org/en-US/docs/DOM/Element.nextElementSibling
> previousElementSibling instead of previousSibling: https://developer.mozilla.org/en-US/docs/DOM/Element.previousElementSibling

and their relatives "firstElementChild", "lastElementChild" :)