contentcollector: Avoid `for..in` iteration of object properties

`for..in` iterates over inherited properties, which is almost never
desired. In most cases there aren't any inherited enumerable
properties so it's not that big of a deal, but in the case of
HTMLCollection it's very bad because it iterates over every entry
twice (once by numerical index and once by name) plus it includes the
`length` property in the iteration.
pull/4685/head
Richard Hansen 2021-01-20 23:50:33 -05:00 committed by John McLear
parent 3cfec58948
commit d0bfb54c0a
1 changed files with 14 additions and 5 deletions

View File

@ -250,8 +250,8 @@ const makeContentCollector = (collectStyles, abrowser, apool, className2Author)
const _recalcAttribString = (state) => {
const lst = [];
for (const a in state.attribs) {
if (state.attribs[a]) {
for (const [a, count] of Object.entries(state.attribs)) {
if (count) {
// The following splitting of the attribute name is a workaround
// to enable the content collector to store key-value attributes
// see https://github.com/ether/etherpad-lite/issues/2567 for more information
@ -496,9 +496,18 @@ const makeContentCollector = (collectStyles, abrowser, apool, className2Author)
// lists do not need to have a type, so before we make a wrong guess
// check if we find a better hint within the node's children
if (!rr && !type) {
for (const i in node.children) {
if (node.children[i] && node.children[i].name === 'ul') {
type = dom.getAttribute(node.children[i], 'class');
// If `node` is from the DOM (not cheerio) then it implements the ParentNode interface
// and `node.children` is a HTMLCollection. The DOM + Web IDL specs guarantee that
// HTMLCollection implements the iterable protocol, so for..of iteration should always
// work. See: https://stackoverflow.com/a/41759532. Cheerio behaves the same with
// regard to iteration.
//
// TODO: The set of Nodes included in node.children differs between the DOM and
// cheerio 0.22.0: cheerio includes all child Nodes (including non-Element Nodes)
// whereas the DOM only includes Nodes that are Elements.
for (const child of node.children) {
if (child && child.name === 'ul') {
type = dom.getAttribute(child, 'class');
if (type) {
break;
}