Avoid live collection

[minas90]

Avoid using childNodes live collection logic and replace with firstChild logic.
This small change fixes compatibility with linkedom

It's already partly fixed with #677 , but some pages still cause infinite loop, like this one: ansa.it

[gijsk]

Hm. As far as I can tell there are still more accesses that are likely to be problematic:

readability/Readability.js

Lines 1175 to 1177 in e197236

	var siblings = parentOfTopCandidate.children;
	for (var s = 0, sl = siblings.length; s < sl; s++) {

(we decrement if append ends up true, and revisit that item in the list)

[minas90]

(we decrement if append ends up true, and revisit that item in the list)

Right, I missed this one.

I pushed a fix. It looks like a hack, but I think it's a good compromise.
The other options are

1. Clone the children array before iterating
2. Fetch children during every iteration

The first way will cause a bit more memory usage.
The second way will have very little affect on jsdom, because children is pre-computed, but it will slow down linkedom, because it's discovering children every time.
The current approach is fetching children again only after it's changed.

Please let me know which approach do you think is better.

Btw is there any good reason to avoid modern javascript syntax to replace this for example?

        while (page.firstChild) {
          this.log("Moving child out:", page.firstChild);
          topCandidate.appendChild(page.firstChild);
        }

with

topCandidate.append(...page.children);

[gijsk]

The current approach is fetching children again only after it's changed.

Please let me know which approach do you think is better.

The current version seems fine.

Btw is there any good reason to avoid modern javascript syntax to replace this for example?

        while (page.firstChild) {
          this.log("Moving child out:", page.firstChild);
          topCandidate.appendChild(page.firstChild);
        }

with

topCandidate.append(...page.children);

The only reason is the minimum node version we support. I'd like to avoid introducing babel or similar preprocessing. If the spread operator is supported in node 10 (which I think it is), then I'd happily take a PR to clean some of this up.

[gijsk]

Thanks!