Make unhide function work as expected in html reporter

[pec9399]

Description

Fixed the iteration of the HTMLCollection object in the unhide function of the html reporter. Since the length of the HTMLCollection changes as the elements' class names get updated (as HTMLCollection works 'live'), it cannot be updated using an index-based for loop.

Description of the Change

Changed the index-based for loop iteration to a while loop, updating the first element while the length of the HTMLCollection is larger than 0.

Why should this be in core?

It a bug (although minor) that makes filtering by passes/failures doesn't work properly as expected.

Benefits

Filtering by passes / filters in html reporter works properly even after unhiding objects.

Possible Drawbacks

None

Applicable issues

Closes #4048

[jsf-clabot]

All committers have signed the CLA.

[juergba]

Could someone test, confirm and review this PR, please? Members, contributors, others ... ?
@Mia-jeong @HyunSangHan @Lindsay-Needs-Sleep

[Lindsay-Needs-Sleep]

@juergba @pec9399
Just tried this out. Works great.

Tested on desktop chrome (77.0.3865.90) and Android (7.0) webview (77.0.3865.92).

Tried the described situations (all passes, all failures), and a mix of passes and fails. All 3 situations worked as expected.

Ran npm run test, all tests passed.

[juergba]

@Lindsay-Needs-Sleep thanks 👍

[juergba]

Since the length of the HTMLCollection changes as the elements' class names get updated (as HTMLCollection works 'live'), [...]

This sounds very weird to me with little understanding of HTML/browsers. But I got so far that:

• document.getElementsByClassName returns an HTMLCollection which is a live list
• document.querySelector returns a NodeList which is a static list

Is hideSuitesWithout() working correctly? As unhide() it's also using getElementsByClassName.

[pec9399]

For clarification, as you noted, document.getElementsByClassName returns an HTMLCollection which is a live list, and therefore changing the class name of an element in the HTMLCollection will remove that element from this collection right away.

In the code below, the code in line 3 will change the ith element's class name to 'suite' and consequently pop the ith element from the variable 'els' since the ith element is not included in document.getElementsByClassName('suite hidden') anymore.

  // original code

  1  var els = document.getElementsByClassName('suite hidden');
  2  for (var i = 0; i < els.length; i++) {
  3    els[i].className = els[i].className.replace('suite hidden', 'suite');
  4  }

This will inevitably change the indices of the remaining elements still in the list, making index-based loops cause unexpected behaviors when iterating over the HTMLCollection. So, I changed the code to change the class name of each element while the list is not empty rather than iterating over each index.

  // new code

  1  var els = document.getElementsByClassName('suite hidden');
  2  while (els.length > 0) {
  3    els[0].className = els[0].className.replace('suite hidden', 'suite');
  4  }

Hope this helps your understanding.

As of hideSuitesWithout(), it doesn't really have this problem. The code in line 6 will change the ith element in the variable 'suites' to 'suites hidden', but it still belongs to document.getElementsByClassName('suite'), so it will not be popped from the original HTMLCollection.

1  function hideSuitesWithout(classname) {
2   var suites = document.getElementsByClassName('suite');
3   for (var i = 0; i < suites.length; i++) {
4     var els = suites[i].getElementsByClassName(classname);
5     if (!els.length) {
6      suites[i].className += ' hidden';
7     }
8   }
9  }

[juergba]

@pec9399 thank you for your detailed explanation.