feat(a11y-query): extend aria handler with waitFor #6472
Conversation
johanbay
requested review from
mathiasbynens,
hanselfmu,
OrKoN and
sadym-chromium
johanbay
force-pushed
the
add-aria-handler-2
branch
from
63fc44d
to
7ec4c62
Compare
test/ariaqueryhandler.spec.ts
Outdated
| ).toBe('anything'); | ||
| }); | ||
|
|
||
| // TODO: Figure out what the stack trace should look like for custom handlers |
There was a problem hiding this comment.
What does it look like currently? Can you make a recommendation?
There was a problem hiding this comment.
This is the current stack trace:
"TimeoutError: waiting for selector `zombo` failed: timeout 10ms exceeded
at new WaitTask (.../puppeteer/lib/cjs/puppeteer/common/DOMWorld.js:494:34)
at DOMWorld.waitForSelectorInPage (.../puppeteer/lib/cjs/puppeteer/common/DOMWorld.js:427:26)
at Object.waitFor (.../puppeteer/lib/cjs/puppeteer/common/AriaQueryHandler.js:55:21)
at process._tickCallback (internal/process/next_tick.js:68:7)"
It would be nice if ariaqueryhandler.spec.ts was part of the trace since that is the actual call site for waitForSelector.
The fact that the selector is zombo instead of aria/zombo is maybe a bit unfortunate as well.
There was a problem hiding this comment.
It seems like we can't do much better than this since waitFor is async.
johanbay
force-pushed
the
add-aria-handler-2
branch
from
7ec4c62
to
7575c5e
Compare
| // Wait for other operation to finish | ||
| if (this._settingUpBinding) { | ||
| await this._settingUpBinding; | ||
| return this.addBindingToContext(name); |
There was a problem hiding this comment.
Would it set up the binding for the same world again?
There was a problem hiding this comment.
No it shouldn't. The first check this._ctxBindings.has(name) should be true if the binding was added by the current operation, so it will return immediately.
The idea behind the recursive call is that if multiple tasks are blocked on await this._settingUpBinding;, then one of them will get to do the recursive call and assign a new promise to _settingUpBinding. When the other tasks gets to do the recursive call they are therefore blocked here again.
johanbay
force-pushed
the
add-aria-handler-2
branch
from
7575c5e
to
24c5bba
Compare
This PR adds
waitForto the built-in aria handler (#6307).This is part 3 of #6453.