fix(region): contents in iframes should pass the region rule if the iframe itself is in a region

[clottman]

If there's an iframe on the page, elements within that iframe should all be treated as being contained in a region if the iframe element was contained on the iframe.

Also adds lots of tests for this behavior.

Closes issue: #2690

[WilcoFiers]

This is also going to cause valid region issues to be ignored. The trick to figure out is how to distinguish these three examples:

<!-- failing h1 -->
<iframe srcdoc="<h1>hello world</h1>"></iframe>

<!-- passing h1 -->
<main>
  <iframe srcdoc="<h1>hello world</h1>"></iframe>
</main>

<!-- passing h1 -->
  <iframe srcdoc="<main>
    <h1>hello world</h1>
</main>"></iframe>

<!-- inapplicable comment -->
<iframe srcdoc="<-- empty frame -->"></iframe>

Axe-core currently fails passing iframe 1, your PR fixes that, but it does so by also passing the failed iframe. I think what we'd need to do is to pass information about which frames are in a region to a new after method, and then in the after method to pass all the content of iframes that are in a region. That'll need to be recursive too.

[clottman]

For this case:

<!-- inapplicable comment -->
<iframe srcdoc="<-- empty frame -->"></iframe>

We can't mark this as inapplicable because we don't know the iframe is empty until after the rule has already run.

[clottman]

This is now done in the after method, so this test won't pass any more.

resolved

[WilcoFiers]

Some cleanup. Rest looks good.

[WilcoFiers]

We should prefer the "exit early" code style.

Suggested change

	if (!r.result) {
	if (r.result) {
	return
	}

[WilcoFiers]

Suggested change

	const ancestryMinusLast = r.node.ancestry.slice(0, -1);
	const frameAncestry = r.node.ancestry.slice(0, -1);

[WilcoFiers]

This feels unnecessarily complex. I suggest you create a list of frames with result:true, and then try to match against that. You have no business running matchAncestry against anything other than iframes. You don't need to do it if ancestry has a length of 1 either.

That also lets you use clearer variable names. It took me a while to figure out what earlierResult was even for.

[clottman]

That strategy won't work because we need to be able to evaluate nested iframes correctly - when an iframe is contained in another iframe that should pass, but the inner iframe does not pass on its own, we don't know until we've iterated sequentially through that the inner iframe should pass and thus be considered part of the set that should be matched against.

Consider this (which is represented in the full integration tests as region-pass-nested-iframe.html):

<main>
  <iframe id='outer' srcdoc="<iframe id='inner' srcdoc='<p>hello</p>'></iframe></iframe>
</main>

Here's the code I was working on, which has the problem described above, if you want to play around with it yourself.

function regionAfter(results) {
  const passingFrameAncestries = results.filter(
    r => r.result && r.data.isIframe
  ).map(r => r.node.ancestry);

  results.forEach((r, index) => {
    // this element failed the check, and it's in an iframe
    if (!r.result && r.node.ancestry.length > 1) {
      // we need to see if it's actually contained in an iframe that passed the check
      const frameAncestry = r.node.ancestry.slice(0, -1);

      for (const frame of passingFrameAncestries) {
        if (matchAncestry(frameAncestry, frame)) {
          r.result = true;
          break;
        }
      }
    }
  });

  // iframe elements should always pass
  results.forEach(r => {
    if (!r.result && r.data.isIframe) {
      r.result = true;
    }
  });
  return results;
}

addressed

[WilcoFiers]

There's a test missing from this, one that I strongly suspect fails.

Suggested change

	});
	});
	it('should pass content if an grandparent frame passes', function() {
	var results = checks.region.after([
	{
	data: { isIframe: true },
	node: {
	ancestry: ['html > body > iframe']
	},
	result: true
	},
	{
	data: { isIframe: true },
	node: {
	ancestry: ['html > body > iframe', 'html > body > iframe']
	},
	result: false
	},
	{
	data: { isIframe: false },
	node: {
	ancestry: [
	'html > body > iframe',
	'html > body > iframe',
	'html > body > p'
	]
	},
	result: false
	}
	]);
	assert.equal(results[0].result, true);
	assert.equal(results[1].result, true);
	assert.equal(results[2].result, true);
	});

[clottman]

There's actually an integration test for this already - region-pass-nested-iframe.html. It's the reason I had to slightly modify your suggested code when I made region-after easier to understand.

addressed