1260025 - Filter large tables with tablefilter hangs since v45.0

Status: RESOLVED WORKSFORME

(Core :: DOM: Core & HTML, defect)

Description

[Marcel]

Attachment: bug-report.tar.gz

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160304114936

Steps to reproduce:

1. Open the HTML file
2. type in the second box "TEST" (for example)


Actual results:

Firefox reports that the script is hanging. 
Affected versions are: 
45.0 a1 (build from 2015-11-02)
45.0.0
45.0.1
48.0 a1 (2016-03-25)


Expected results:

The script should have filtered the rows and show an empty table.
Works fine with:
38.7.1
44.0.2
45.0 a1 (build from 2015-11-01)

Comment 1

[Loic]

Attachment: testcase.html (self-contained testcase)

Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=798c82c8a96d1099cf712434d9c9036fc48d8297&tochange=9160f08660b8290559e427fd80d617edd86fe2a6

Suspected: bug 264412.

Comment 2

[Loic]

Mats, as roc is away, could you find someone in charge of this part of code, please.

Comment 3

[Mats Palmgren (inactive)]

Here's the problematic script:

function tf_GetNodeText(n)
/*====================================================
	- returns text + text of child nodes of a node
=====================================================*/
{
	var s = n.textContent || n.innerText || n.innerHTML.replace(/\<[^<>]+>/g, '');
	s = s.replace(/^\s+/, '').replace(/\s+$/, '')/*.tf_Trim()*/;
	return s.tf_Trim();
}

So when n.textContent returns "" we will evaluate n.innerText which in this case
also returns "" so we will evaluate n.innerHTML too.  This function is called
once for each <td> it seems, about half of which are empty.

Apart from that, n.innerText flushes layout(*) so if there are DOM / style changes
between the calls to tf_GetNodeText this will be very costly.  And this is what
causes the performance problem in this case.

(*) this is documented here:
https://developer.mozilla.org/en-US/docs/Web/API/Node/textContent

So, I think this bug is invalid.  The workaround to the .innerText flush
problem is to set display:none on the subtree while it's being modified.
In this case I'm guessing it's the <table> element. Like so:
tableElement.style.display="none"
// sort the table, i.e. all the tf_GetNodeText calls happens here
tableElement.style.display=""

Comment 4

[Loic]

.innerText is called in tablefilter_all.js, so is it a bug in this JS lib?

Comment 5

[Mats Palmgren (inactive)]

Yes.

Comment 6

[Loic]

Is it possible for you to file a bug report as you know the subject better than me?
https://github.com/koalyptus/TableFilter/issues

Comment 7

[Mats Palmgren (inactive)]

I'll leave that for someone else.  Comment 3 have all the info needed about the problem.

Comment 8

[Karl Dubost💡 :karlcow]

done https://github.com/koalyptus/TableFilter/issues/162

Comment 9

[Marcel]

The issue was fixed by the JS lib by not executing .innerText if .textContent is supported.
Thanks for the fast response! Closing this bug as it works with the current version now.