WordPress / theme-review-action

Other
30 stars 9 forks source link

TwentyTwenty - DOMException: play() failed because the user didn't interact with the document first. #65

Closed danzinger closed 2 years ago

danzinger commented 2 years ago

I run theme-review-action locally on the Twenty Twenty Theme v1.9 and I get:

User Interface Errors:
"/" (via: index.php) contains javascript errors. Found Error: DOMException: play() failed because the user didn't interact with the document first. https://goo.gl/xX8pDD
See: https://github.com/WordPress/theme-review-action/blob/trunk/docs/ui-errors.md#browser-console-should-not-contain-errors

"/?cat=1" (via: index.php) contains javascript errors. Found Error: DOMException: play() failed because the user didn't interact with the document first. https://goo.gl/xX8pDD
See: https://github.com/WordPress/theme-review-action/blob/trunk/docs/ui-errors.md#browser-console-should-not-contain-errors

"/?tag=alignment-2" (via: index.php) contains javascript errors. Found Error: DOMException: play() failed because the user didn't interact with the document first. https://goo.gl/xX8pDD
See: https://github.com/WordPress/theme-review-action/blob/trunk/docs/ui-errors.md#browser-console-should-not-contain-errors

"/?post_format=gallery" (via: index.php) contains javascript errors. Found Error: DOMException: play() failed because the user didn't interact with the document first. https://goo.gl/xX8pDD
See: https://github.com/WordPress/theme-review-action/blob/trunk/docs/ui-errors.md#browser-console-should-not-contain-errors

I struggle to find the cause of these errors because I'm working on a theme that also gets these errors. Twenty Twenty One however does not have these errors. I know these errors are triggered if autoplay is set e.g on a <video> element and not muted. Such errors are actually content-related but there must be some way to prevent them from beeing triggered when writing and testing a theme.

Could anyone give me a hint on how to prevent these errors?

StevenDufresne commented 2 years ago

Hi @danzinger!

This project is designed to test on pre-existing (curated) data: https://raw.githubusercontent.com/wpaccessibility/a11y-theme-unit-test/master/a11y-theme-unit-test-data.xml

If you are testing on your own data, I would suggest you spin up the environment using:

https://github.com/WordPress/theme-review-action/blob/64554690d3e355c34530111cdeaa1d766b8cae11/package.json#L10

And navigate to /?post_format=gallery to take a look.

Afterwards, you can clean up by running npm run wp-env destroy. That will delete the docker instances that were created on install.

danzinger commented 2 years ago

Hi @StevenDufresne ! Possible missunderstanding, sorry. I dont want to use other test data, all tests use the official test data. I am wondering if this might be a bug with the theme-review-action because the official TwentyTwenty theme should not have these errors, right? If you navigate to these URLs in Chrome manually you dont see these errors in the console but theme-review-action reports them.

But since these errors are actually content related, as explained here, I am wondering how to prevent them when writing a theme. What does TwentyTwentyOne makes right, what TwentyTwenty does not make right? Or might this be some weired bug with the theme-review-action? However, theme-review-action should not report console errors when there actually aren't any console errors, thats why I created the issue here.

StevenDufresne commented 2 years ago

Got it. At quick glance, it looks like this is the result of content indeed. I don't know why it only appears for twenty twenty, running tests headless. Most likely related to the way puppeteer (the framework we use) waits, or doesn't wait for resources to load. I suspect this would also be intermittent for other nonoffending themes.

Most likely the video embeds: https://github.com/wpaccessibility/a11y-theme-unit-test/blob/5c5a16468b34fe46c6626839c795abc2cb00adc9/a11y-theme-unit-test-data.xml#L8914

Since the UI checks are still considered non-blocking, you can disregard this error for the time being.

danzinger commented 2 years ago

Yes, I also thought about puppeteer might cause related problems as there is no "real" user interaction before some play() starts, so these errors are reported. The error clearly states this lack of user interaction: Found Error: DOMException: play() failed because the user didn't interact with the document first. Maybe puppeteer could interact with the document before the console errors are checked. Simple interactions like a simulated click or tap event could help.

I think it would be good to resolve this before ui-errors become a blocker as proposed here .

StevenDufresne commented 2 years ago

Yep, I looked into providing browser flags and also tried to select the document early but wasn't successful. Since we wait for networkidle2 event it's too late, it seems. However, networkidle2 event makes other tests more stable and I don't suggest we change that just yet.

I agree that this shouldn't be a blocker. We'll leave this ticket open and should it become a more widespread issue or we agree on making this an upload block, we can probably just use a more curated test data set.

StevenDufresne commented 2 years ago

I decided to stop fetching the a11y database and keep a local copy without the embeds. The errors should go away now.

See #68.